Use HashMap for literals look up (#2960) (#3277)

987Nabil · web-flow · commit d8f9c6969d49 · 2025-01-22T11:58:57.000-05:00
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -636,6 +636,44 @@ jobs:
           name: Jmh_Main_ProbeContentTypeBenchmark
           path: Main_ProbeContentTypeBenchmark.txt
 
+  Jmh_RoutesBenchmark:
+    name: Jmh RoutesBenchmark
+    if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest]
+        scala: [2.13.14]
+        java: [temurin@8]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: coursier/setup-action@v1
+        with:
+          apps: sbt
+
+      - uses: actions/checkout@v4
+        with:
+          path: zio-http
+
+      - uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: 11
+
+      - name: Benchmark_Main
+        id: Benchmark_Main
+        env:
+          GITHUB_TOKEN: ${{secrets.ACTIONS_PAT}}
+        run: |
+          cd zio-http
+          sed -i -e '$aaddSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.7")' project/plugins.sbt
+          cat > Main_RoutesBenchmark.txt
+          sbt -no-colors -v "zioHttpBenchmarks/jmh:run -i 3 -wi 3 -f1 -t1 RoutesBenchmark" | grep -e "thrpt" -e "avgt" >> ../Main_RoutesBenchmark.txt
+
+      - uses: actions/upload-artifact@v4
+        with:
+          name: Jmh_Main_RoutesBenchmark
+          path: Main_RoutesBenchmark.txt
+
   Jmh_SchemeDecodeBenchmark:
     name: Jmh SchemeDecodeBenchmark
     if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
@@ -752,7 +790,7 @@ jobs:
 
   Jmh_cache:
     name: Cache Jmh benchmarks
-    needs: [Jmh_CachedDateHeaderBenchmark, Jmh_ClientBenchmark, Jmh_CookieDecodeBenchmark, Jmh_EndpointBenchmark, Jmh_HttpCollectEval, Jmh_HttpCombineEval, Jmh_HttpNestedFlatMapEval, Jmh_HttpRouteTextPerf, Jmh_ProbeContentTypeBenchmark, Jmh_SchemeDecodeBenchmark, Jmh_ServerInboundHandlerBenchmark, Jmh_UtilBenchmark]
+    needs: [Jmh_CachedDateHeaderBenchmark, Jmh_ClientBenchmark, Jmh_CookieDecodeBenchmark, Jmh_EndpointBenchmark, Jmh_HttpCollectEval, Jmh_HttpCombineEval, Jmh_HttpNestedFlatMapEval, Jmh_HttpRouteTextPerf, Jmh_ProbeContentTypeBenchmark, Jmh_RoutesBenchmark, Jmh_SchemeDecodeBenchmark, Jmh_ServerInboundHandlerBenchmark, Jmh_UtilBenchmark]
     if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
     strategy:
       matrix:
@@ -824,6 +862,13 @@ jobs:
       - name: Format_Main_ProbeContentTypeBenchmark
         run: cat Main_ProbeContentTypeBenchmark.txt >> Main_benchmarks.txt
 
+      - uses: actions/download-artifact@v4
+        with:
+          name: Jmh_Main_RoutesBenchmark
+
+      - name: Format_Main_RoutesBenchmark
+        run: cat Main_RoutesBenchmark.txt >> Main_benchmarks.txt
+
       - uses: actions/download-artifact@v4
         with:
           name: Jmh_Main_SchemeDecodeBenchmark
diff --git a/profiling/build.sbt b/profiling/build.sbt
@@ -1,7 +1,7 @@
 name         := "zio-http"
 version      := "1.0.0"
 scalaVersion := "2.13.6"
-lazy val zhttp = ProjectRef(file("/zio-http/zio-http-src"), "zioHttp")
+lazy val zioHttp = ProjectRef(file("/zio-http/zio-http-src"), "zioHttp")
 lazy val root  = (project in file("."))
   .settings(
     name                             := "helloExample",
@@ -14,4 +14,4 @@ lazy val root  = (project in file("."))
         oldStrategy(x)
     },
   )
-  .dependsOn(zhttp)
+  .dependsOn(zioHttp)
diff --git a/profiling/project/build.properties b/profiling/project/build.properties
@@ -1 +1 @@
-sbt.version = 1.8.3
+sbt.version = 1.10.7
diff --git a/profiling/project/plugins.sbt b/profiling/project/plugins.sbt
@@ -1,3 +1,3 @@
-addSbtPlugin("com.eed3si9n"  % "sbt-assembly" % "1.1.0")
-addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.11.0")
-addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.0")
+addSbtPlugin("com.eed3si9n"  % "sbt-assembly" % "1.2.0")
+addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.14.0")
+addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.4")
diff --git a/zio-http-benchmarks/src/main/scala/zio/http/benchmark/RoutesBenchmark.scala b/zio-http-benchmarks/src/main/scala/zio/http/benchmark/RoutesBenchmark.scala
@@ -0,0 +1,77 @@
+package zio.http.benchmark
+
+import java.util.concurrent.TimeUnit
+
+import scala.util.Random
+
+import zio.{Runtime, Unsafe, ZIO}
+
+import zio.http.endpoint.Endpoint
+import zio.http.{Handler, Method, Request, Routes}
+
+import org.openjdk.jmh.annotations._
+
+@State(Scope.Thread)
+@BenchmarkMode(Array(Mode.Throughput))
+@OutputTimeUnit(TimeUnit.SECONDS)
+class RoutesBenchmark {
+
+  val REPEAT_N = 1000
+
+  val paths = ('a' to 'z').inits.map(_.mkString).toList.reverse.tail
+
+  val routes = Routes.fromIterable(paths.map(p => Endpoint(Method.GET / p).out[Unit].implementHandler(Handler.unit)))
+
+  val requests = paths.map(p => Request.get(p))
+
+  def request: Request  = requests(Random.nextInt(requests.size))
+  val smallDataRequests = Array.fill(REPEAT_N)(request)
+
+  def unsafeRun[E, A](zio: ZIO[Any, E, A]): Unit = Unsafe.unsafe { implicit unsafe =>
+    Runtime.default.unsafe
+      .run(zio.unit)
+      .getOrThrowFiberFailure()
+  }
+
+  val paths2 = ('b' to 'z').inits.map(_.mkString).toList.reverse.tail
+
+  val routes2 = Routes.fromIterable(paths2.map(p => Endpoint(Method.GET / p).out[Unit].implementHandler(Handler.unit)))
+
+  val requests2 = requests ++ paths2.map(p => Request.get(p))
+
+  def request2: Request = requests2(Random.nextInt(requests2.size))
+
+  val smallDataRequests2 = Array.fill(REPEAT_N)(request2)
+
+  val routes3 = Routes(
+    Endpoint(Method.GET / "api").out[Unit].implementHandler(Handler.unit),
+    Endpoint(Method.GET / "ui").out[Unit].implementHandler(Handler.unit),
+  )
+
+  val requests3 = Array.fill(REPEAT_N)(List(Request.get("api"), Request.get("ui"))(Random.nextInt(2)))
+
+  @Benchmark
+  def benchmarkSmallDataZioApi(): Unit =
+    for (r <- smallDataRequests) routes.isDefinedAt(r)
+
+  @Benchmark
+  def benchmarkSmallDataZioApi2(): Unit =
+    for (r <- smallDataRequests2) routes2.isDefinedAt(r)
+
+  @Benchmark
+  def notFound1(): Unit =
+    for (_ <- 1 to REPEAT_N) {
+      routes.isDefinedAt(Request.get("not-found"))
+    }
+
+  @Benchmark
+  def notFound2(): Unit =
+    for (_ <- 1 to REPEAT_N) {
+      routes2.isDefinedAt(Request.get("not-found"))
+    }
+
+  @Benchmark
+  def benchmarkSmallDataZioApi3(): Unit =
+    for (r <- requests3) routes3.isDefinedAt(r)
+
+}
diff --git a/zio-http/jvm/src/test/scala/zio/http/RoutePatternSpec.scala b/zio-http/jvm/src/test/scala/zio/http/RoutePatternSpec.scala
@@ -230,9 +230,10 @@ object RoutePatternSpec extends ZIOHttpSpec {
         var tree: Tree[Int] = RoutePattern.Tree.empty
 
         val pattern1 = Method.GET / "users" / "123"
-        val pattern2 = Method.GET / "users" / trailing
+        val pattern2 = Method.GET / "users" / trailing / "123"
 
         tree = tree.add(pattern2, 2)
+        println(tree.get(Method.GET, Path("/users/bla/123")))
         tree = tree.add(pattern1, 1)
 
         assertTrue(tree.get(Method.GET, Path("/users/123")).contains(1))
diff --git a/zio-http/shared/src/main/scala/zio/http/codec/PathCodec.scala b/zio-http/shared/src/main/scala/zio/http/codec/PathCodec.scala
@@ -17,7 +17,7 @@
 package zio.http.codec
 
 import scala.annotation.tailrec
-import scala.collection.immutable.ListMap
+import scala.collection.immutable.{HashMap, ListMap}
 import scala.collection.mutable
 import scala.language.implicitConversions
 
@@ -763,7 +763,7 @@ object PathCodec {
   }
 
   private[http] final case class SegmentSubtree[+A](
-    literals: ListMap[String, SegmentSubtree[A]],
+    literals: Map[String, SegmentSubtree[A]],
     others: ListMap[SegmentCodec[_], SegmentSubtree[A]],
     literalsWithCollisions: Set[String],
     value: Chunk[A],
@@ -778,8 +778,8 @@ object PathCodec {
         newOthers.keys,
       )
       SegmentSubtree(
-        newLiterals,
-        newOthers,
+        Map(newLiterals.toList: _*),
+        ListMap(newOthers.toList: _*),
         newLiteralCollisions,
         self.value ++ that.value,
       )
@@ -791,7 +791,7 @@ object PathCodec {
     def get(path: Path): Chunk[A] =
       get(path, 0)
 
-    private def get(path: Path, from: Int, skipLiteralsFor: Set[Int] = null): Chunk[A] = {
+    private def get(path: Path, from: Int, skipLiteralsFor: Set[Int] = Set.empty): Chunk[A] = {
       val segments  = path.segments
       val nSegments = segments.length
       var subtree   = self
@@ -804,7 +804,10 @@ object PathCodec {
         val segment = segments(i)
 
         // Fast path, jump down the tree:
-        if ((skipLiteralsFor.eq(null) || !skipLiteralsFor.contains(i)) && subtree.literals.contains(segment)) {
+        if (
+          subtree.literals.contains(segment)
+          && (subtree.literalsWithCollisions.eq(Set.empty) || !skipLiteralsFor.contains(i))
+        ) {
 
           // this subtree segment have conflict with others
           // will try others if result was empty
@@ -830,7 +833,7 @@ object PathCodec {
                 result = subtree0.value
                 i += matched
               }
-            case n => // Slowest fallback path. Have to to find the first predicate where the subpath returns a result
+            case n => // Slowest fallback path. Have to find the first predicate where the subpath returns a result
               val matches         = Array.ofDim[Int](n)
               var index           = 0
               var nPositive       = 0
@@ -886,11 +889,10 @@ object PathCodec {
 
       if (trySkipLiteralIdx.nonEmpty && result.isEmpty) {
         trySkipLiteralIdx = trySkipLiteralIdx.reverse
-        val skipLiteralsFor0 = if (skipLiteralsFor eq null) Set.empty[Int] else skipLiteralsFor
         while (trySkipLiteralIdx.nonEmpty && result.isEmpty) {
           val skipIdx = trySkipLiteralIdx.head
           trySkipLiteralIdx = trySkipLiteralIdx.tail
-          result = get(path, from, skipLiteralsFor0 + skipIdx)
+          result = get(path, from, skipLiteralsFor + skipIdx)
         }
         result
       } else result
@@ -914,7 +916,7 @@ object PathCodec {
   object SegmentSubtree    {
     def single[A](segments: Iterable[SegmentCodec[_]], value: A): SegmentSubtree[A] =
       segments.collect { case x if x.nonEmpty => x }
-        .foldRight[SegmentSubtree[A]](SegmentSubtree(ListMap(), ListMap(), Set.empty, Chunk(value))) {
+        .foldRight[SegmentSubtree[A]](SegmentSubtree(Map.empty, ListMap(), Set.empty, Chunk(value))) {
           case (segment, subtree) =>
             val literals =
               segment match {
@@ -928,14 +930,14 @@ object PathCodec {
                 case _                       => Chunk((segment, subtree))
               }): _*)
 
-            SegmentSubtree(literals, others, Set.empty, Chunk.empty)
+            SegmentSubtree(Map(literals.toList: _*), others, Set.empty, Chunk.empty)
         }
 
     val empty: SegmentSubtree[Nothing] =
-      SegmentSubtree(ListMap(), ListMap(), Set.empty, Chunk.empty)
+      SegmentSubtree(Map(), ListMap(), Set.empty, Chunk.empty)
   }
 
-  private def mergeMaps[A, B](left: ListMap[A, B], right: ListMap[A, B])(f: (B, B) => B): ListMap[A, B] =
+  private def mergeMaps[A, B](left: Map[A, B], right: Map[A, B])(f: (B, B) => B): Map[A, B] =
     right.foldLeft(left) { case (acc, (k, v)) =>
       acc.get(k) match {
         case None     => acc.updated(k, v)

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-sbt.version = 1.8.3`
	`1`	`+sbt.version = 1.10.7`