finagle-http: Optimize HttpMuxer.normalize

Problem

`c.t.f.http.HttpMuxer.normalize` is slower than it could be.

Solution

Optimize it.

Result

Significantly faster and far less allocations. The common case when
the path is already normalized is 96% faster and has 0 allocations.

JIRA Issues: CSL-8520

Differential Revision: https://phabricator.twitter.biz/D343261

finagle-benchmark/src/main/scala/com/twitter/finagle/http/HttpMuxerBenchmark.scala

@@ -0,0 +1,52 @@
+package com.twitter.finagle.http
+
+import com.twitter.finagle.benchmark.StdBenchAnnotations
+import org.openjdk.jmh.annotations.{Benchmark, Scope, State}
+
+@State(Scope.Benchmark)
+class HttpMuxerBenchmark extends StdBenchAnnotations {
+
+  private[this] var i = 0
+
+  private[this] val alreadyNormalized = IndexedSeq(
+    "/foo/bar",
+    "/cool/cool/cool",
+    "/"
+  )
+
+  private[this] val excessiveSlashes = IndexedSeq(
+    "/foo///bar",
+    "/cool//cool////cool",
+    "////"
+  )
+
+  private[this] val missingLeadingSlash = IndexedSeq(
+    "foo/bar",
+    "cool/cool/cool"
+  )
+
+  @Benchmark
+  def normalize_alreadyNormalized: String = {
+    i += 1
+    if (i < 0) i = 0
+    val path = alreadyNormalized(i % alreadyNormalized.size)
+    HttpMuxer.normalize(path)
+  }
+
+  @Benchmark
+  def normalize_excessiveSlashes: String = {
+    i += 1
+    if (i < 0) i = 0
+    val path = excessiveSlashes(i % excessiveSlashes.size)
+    HttpMuxer.normalize(path)
+  }
+
+  @Benchmark
+  def normalize_missingLeadingSlash: String = {
+    i += 1
+    if (i < 0) i = 0
+    val path = missingLeadingSlash(i % missingLeadingSlash.size)
+    HttpMuxer.normalize(path)
+  }
+
+}

finagle-http/src/main/scala/com/twitter/finagle/http/HttpMuxer.scala

@@ -2,8 +2,9 @@ package com.twitter.finagle.http
 
 import com.twitter.finagle.util.LoadService
 import com.twitter.finagle.Service
-import com.twitter.util.{Future, Time, Closable}
+import com.twitter.util.{Closable, Future, Time}
 import java.util.logging.Logger
+import scala.annotation.tailrec
 
 /**
  * A service that dispatches incoming requests to registered handlers.
@@ -23,6 +24,7 @@ import java.util.logging.Logger
  *  NOTE: When multiple pattern matches exist, the longest pattern wins.
  */
 class HttpMuxer(_routes: Seq[Route]) extends Service[Request, Response] {
+  import HttpMuxer.normalize
 
   def this() = this(Seq.empty[Route])
 
@@ -77,18 +79,6 @@ class HttpMuxer(_routes: Seq[Route]) extends Service[Request, Response] {
     case None => Future.value(Response(request.version, Status.NotFound))
   }
 
-  /**
-   * - ensure path starts with "/"
-   * - get rid of excessive "/"s. For example "/a//b///c/" => "/a/b/c/"
-   * - return "" if path is ""
-   * - return "/" if path is "/" or "///" etc
-   */
-  private[this] def normalize(path: String) = {
-    val suffix = if (path.endsWith("/")) "/" else ""
-    val p = path.split("/").filterNot(_.isEmpty).mkString("/")
-    if (p == "") suffix else "/" + p + suffix
-  }
-
   override def close(deadline: Time): Future[Unit] =
     Closable.all(routes.map(_.handler): _*).close(deadline)
 }
@@ -101,6 +91,45 @@ class HttpMuxer(_routes: Seq[Route]) extends Service[Request, Response] {
 object HttpMuxer extends HttpMuxer {
   @volatile private[this] var underlying = new HttpMuxer()
 
+  private[this] def addLeadingAndStripDuplicateSlashes(s: String): String = {
+    val b = new java.lang.StringBuilder(s.length)
+    b.append('/')
+
+    @tailrec
+    def go(i: Int, lastSlash: Boolean): Unit = {
+      if (i < s.length) {
+        val c = s.charAt(i)
+        if (lastSlash && c == '/') go(i + 1, true)
+        else {
+          b.append(c)
+          go(i + 1, c == '/')
+        }
+      }
+    }
+    go(0, true)
+
+    b.toString
+  }
+
+  /**
+   * - ensure path starts with "/" (unless path is "")
+   * - get rid of excessive "/"s. For example "/a//b///c/" => "/a/b/c/"
+   * - return "" if path is ""
+   * - return "/" if path is "/" or "///" etc
+   *
+   * @note exposed for testing.
+   */
+  private[http] def normalize(path: String): String = {
+    if (path.isEmpty) {
+      ""
+    } else if (!path.contains("//")) {
+      if (path.startsWith("/")) path
+      else "/" + path
+    } else {
+      addLeadingAndStripDuplicateSlashes(path)
+    }
+  }
+
   /**
    * add handlers to mutate dispatching strategies.
    */

finagle-http/src/test/scala/com/twitter/finagle/http/HttpMuxerTest.scala

@@ -32,6 +32,20 @@ class HttpMuxerTest extends FunSuite {
     assert(Await.result(muxService(Request("/foo/bar/blah?j={}"))).contentString == fooBarPrefix)
   }
 
+  test("normalize basics") {
+    assert(HttpMuxer.normalize("") == "")
+    assert(HttpMuxer.normalize("/") == "/")
+    assert(HttpMuxer.normalize("/foo") == "/foo")
+    assert(HttpMuxer.normalize("/foo/") == "/foo/")
+    assert(HttpMuxer.normalize("foo") == "/foo")
+  }
+
+  test("normalize duplicate slashes") {
+    assert(HttpMuxer.normalize("/foo//bar") == "/foo/bar")
+    assert(HttpMuxer.normalize("///foo//bar") == "/foo/bar")
+    assert(HttpMuxer.normalize("/foo//bar///") == "/foo/bar/")
+  }
+
   test("prefix matching is handled correctly") {
     assert(await(muxService(Request("/foo/<a>"))).contentString == percentEncode)