finagle-base-http: add MapBackedHeaderMap

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/DefaultHeaderMap.scala

@@ -3,5 +3,5 @@ package com.twitter.finagle.http.headers
 import com.twitter.finagle.http.HeaderMap
 
 object DefaultHeaderMap {
-  def apply(headers: (String, String)*): HeaderMap = HashBackedHeaderMap(headers: _*)
+  def apply(headers: (String, String)*): HeaderMap = JTreeMapBackedHeaderMap(headers: _*)
 }

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/HashBackedHeaderMap.scala

@@ -82,7 +82,7 @@ private final class HashBackedHeaderMap extends http.HeaderMap {
     }
 }
 
-private object HashBackedHeaderMap {
+object HashBackedHeaderMap {
 
   /** Construct a new `HeaderMap` with the header list
    *

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/Header.scala

@@ -1,7 +1,8 @@
 package com.twitter.finagle.http.headers
 
-import com.twitter.finagle.http.HeaderMap
 import scala.collection.mutable
+import scala.collection.AbstractIterator
+import com.twitter.finagle.http.HeaderMap
 
 private[http] sealed class Header private (final val name: String, final val value: String)
     extends HeaderMap.NameValue {
@@ -15,6 +16,20 @@ private[http] object Header {
 
   final class Root private[Header] (name: String, value: String) extends Header(name, value) {
 
+    def iterator: Iterator[HeaderMap.NameValue] =
+      if (next == null) Iterator.single(this)
+      else {
+        var cur: Header = this
+        new AbstractIterator[HeaderMap.NameValue] {
+          def hasNext: Boolean = cur != null
+          def next(): HeaderMap.NameValue = {
+            var n = cur
+            cur = n.next
+            n
+          }
+        }
+      }
+
     // We want to keep a reference to the last element of this linked list
     // so we don't need to traverse the whole list to add an element.
     private[this] var last: Header = null

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/HeadersHash.scala

@@ -9,10 +9,7 @@ import scala.collection.mutable
 // - iteration by gaining access to `entriesIterator` (protected method).
 // - get/add functions by providing custom hashCode and equals methods for a key
 private[http] final class HeadersHash extends mutable.HashMap[String, Header.Root] {
-
-  private def hashChar(c: Char): Int =
-    if (c >= 'A' && c <= 'Z') c + 32
-    else c
+  import HeadersHash._
 
   // Adopted from Netty 3 HttpHeaders.
   override protected def elemHashCode(key: String): Int = {
@@ -181,3 +178,9 @@ private[http] final class HeadersHash extends mutable.HashMap[String, Header.Roo
   def removeAll(key: String): Unit = remove(key)
 
 }
+
+object HeadersHash {
+  final def hashChar(c: Char): Int =
+    if (c >= 'A' && c <= 'Z') c + 32
+    else c
+}

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/JMapBackedHeaderMap.scala

@@ -0,0 +1,99 @@
+package com.twitter.finagle.http.headers
+
+import com.twitter.finagle.http.HeaderMap
+import java.util.function.BiConsumer
+import scala.collection.AbstractIterator
+
+/**
+ * Mutable, thread-safe [[HeaderMap]] implementation, backed by
+ * a mutable [[Map[String, Header]]], where the map key
+ * is forced to lower case
+ */
+private[http] trait JMapBackedHeaderMap extends HeaderMap {
+  import HeaderMap._
+
+  // In general, Map's that are not thread safe are not
+  // durable to concurrent modification and can result in infinite loops
+  // and exceptions.
+  // As such, we synchronize on the underlying collection when performing
+  // accesses to avoid this. In the common case of no concurrent access,
+  // this should be cheap.
+  protected val underlying: java.util.Map[String, Header.Root]
+
+  private def foreachConsumer[U](f: ((String, String)) => U):
+    BiConsumer[String, Header.Root] = new BiConsumer[String, Header.Root](){
+      def accept(key: String, header: Header.Root): Unit = header.iterator.foreach(
+        nv => f(nv.name, nv.value)
+      )
+    }
+
+  final override def foreach[U](f: ((String, String)) => U): Unit = 
+    underlying.forEach(foreachConsumer(f))
+
+  // ---- HeaderMap -----
+
+  // Validates key and value.
+  final def add(key: String, value: String): this.type = {
+    validateName(key)
+    addUnsafe(key, foldReplacingValidateValue(key, value))
+  }
+
+  // Does not validate key and value.
+  def addUnsafe(key: String, value: String): this.type
+
+  // Validates key and value.
+  final def set(key: String, value: String): this.type = {
+    validateName(key)
+    setUnsafe(key, foldReplacingValidateValue(key, value))
+  }
+
+  // Does not validate key and value.
+  def setUnsafe(key: String, value: String): this.type
+
+  // ---- Map/MapLike -----
+
+  def -=(key: String): this.type = removed(key)
+  def +=(kv: (String, String)): this.type = set(kv._1, kv._2)
+
+
+  def get(key: String): Option[String]
+
+  /**
+   * Underlying headers eagerly copied to an array, without synchronizing
+   * on the underlying collection.
+   */
+  private[this] def copyHeaders: Iterator[Header.Root] = underlying.values.toArray(new Array[Header.Root](underlying.size)).iterator
+
+  final def iterator: Iterator[(String, String)] = underlying.synchronized {
+    copyHeaders.flatMap(_.iterator.map(nv => (nv.name, nv.value)))
+  }
+
+  def removed(key: String): this.type
+
+  final override def keys: Set[String] = keysIterator.toSet
+
+  final override def keysIterator: Iterator[String] = underlying.synchronized {
+    //the common case has a single element in Headers. Prevent unneeded List
+    //allocations for that case (don't flatMap)
+    val valuesIterator = copyHeaders
+    var currentEntries: Iterator[String] = Iterator.empty
+    new AbstractIterator[String]{
+      def hasNext: Boolean = currentEntries.hasNext || valuesIterator.hasNext
+      def next(): String =
+        if (currentEntries.hasNext) currentEntries.next()
+        else {
+          val h = valuesIterator.next()
+          if (h.next == null) h.name
+          else {
+            currentEntries = h.iterator.map(nv => nv.name).toList.distinct.iterator
+            currentEntries.next()
+          }
+        }
+    }
+  }
+
+  private[finagle] final override def nameValueIterator: Iterator[HeaderMap.NameValue] =
+    underlying.synchronized {
+      copyHeaders.flatMap(_.iterator)
+    }
+}

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/JTreeMapBackedHeaderMap.scala

@@ -0,0 +1,79 @@
+package com.twitter.finagle.http.headers
+
+import com.twitter.finagle.http.HeaderMap
+import scala.annotation.tailrec
+
+/**
+ * Mutable, thread-safe [[HeaderMap]] implementation, backed by
+ * a mutable [[Map[String, Header]]], where the map key
+ * is forced to lower case
+ */
+final private[http] class JTreeMapBackedHeaderMap extends JMapBackedHeaderMap {
+
+  override val underlying: java.util.TreeMap[String, Header.Root] = 
+    new java.util.TreeMap[String, Header.Root](JTreeMapBackedHeaderMap.SharedComparitor)
+
+    def getAll(key: String): Seq[String] = underlying.synchronized {
+      underlying.get(key.toLowerCase) match {
+        case null => Nil
+        case r: Header.Root => r.values
+      }
+    }
+
+  // Does not validate key and value.
+  def addUnsafe(key: String, value: String): this.type  = underlying.synchronized {
+    def header = Header.root(key, value)
+    underlying.get(key) match {
+      case null => underlying.put(key, header)
+      case h    => h.add(key, value)
+    }
+    this
+  }
+
+  // Does not validate key and value.
+  def setUnsafe(key: String, value: String): this.type = underlying.synchronized {
+    underlying.put(key, Header.root(key, value))
+    this
+  }
+
+  def get(key: String): Option[String] = underlying.synchronized {
+    Option(underlying.get(key)).map(_.value)
+  }
+
+  def removed(key: String): this.type = underlying.synchronized {
+    underlying.remove(key)
+    this
+  }
+}
+
+
+object JTreeMapBackedHeaderMap {
+
+  val SharedComparitor = new java.util.Comparator[String] {
+    def compare(key1: String, key2: String): Int = {
+      // Shorter strings are always less, regardless of their content
+      val lenthDiff = key1.length - key2.length
+      if (lenthDiff != 0) lenthDiff
+      else {
+        @tailrec
+        def go(i: Int): Int = {
+          if (i == key1.length) 0 // end, they are equal.
+          else {
+            val char1 = HeadersHash.hashChar(key1.charAt(i))
+            val char2 = HeadersHash.hashChar(key2.charAt(i))
+            val diff = char1 - char2
+            if (diff == 0) go(i + 1)
+            else diff
+          }
+        }
+        go(0)
+      }
+    }
+  }
+
+  def apply(headers: (String, String)*): HeaderMap = {
+    val result = new JTreeMapBackedHeaderMap
+    headers.foreach(t => result.add(t._1, t._2))
+    result
+  }
+}

finagle-base-http/src/test/scala/com/twitter/finagle/http/headers/DefaultHeaderMapTest.scala

@@ -2,6 +2,10 @@ package com.twitter.finagle.http.headers
 
 import com.twitter.finagle.http.HeaderMap
 
-class DefaultHeaderMapTest extends AbstractHeaderMapTest {
-  final def newHeaderMap(headers: (String, String)*): HeaderMap = DefaultHeaderMap(headers: _*)
+class HashBackedHeaderMapTest extends AbstractHeaderMapTest {
+  final def newHeaderMap(headers: (String, String)*): HeaderMap = HashBackedHeaderMap(headers: _*)
 }
+
+class JTreeMapBackedHeaderMapTest extends AbstractHeaderMapTest {
+  final def newHeaderMap(headers: (String, String)*): HeaderMap = JTreeMapBackedHeaderMap(headers: _*)
+}

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

@@ -1,6 +1,10 @@
 package com.twitter.finagle.http
 
 import com.twitter.finagle.benchmark.StdBenchAnnotations
+import com.twitter.finagle.http.headers.{
+  JTreeMapBackedHeaderMap,
+  HashBackedHeaderMap
+}
 import org.openjdk.jmh.annotations._
 import org.openjdk.jmh.infra.Blackhole
 import scala.util.Random
@@ -13,7 +17,7 @@ abstract class HeaderMapBenchmark extends StdBenchAnnotations {
   // We supply 18 random strings of the length of 14 and build a 9-element
   // header map of them. The 10th element is foo -> bar so we can reliably
   // query it in the benchmark.
-  private val map = Iterator
+  private val randommap = Iterator
     .fill(9 * 2)(Random.alphanumeric.take(14).mkString)
     .grouped(2)
     .foldLeft(newMap())((map, h) => map.add(h.head, h.last))
@@ -22,35 +26,98 @@ abstract class HeaderMapBenchmark extends StdBenchAnnotations {
   private val duplicateKeyMap =
     (0 until 10).foldLeft(newMap())((map, _) => map.add("key'", "value"))
 
+  private val nonrandommap = newMap()
+    .add("access-control-allow-credentials", "true")
+    .add("access-control-allow-origin", "https://twitter.com")
+    .add("cache-control", "no-cache, no-store, must-revalidate, pre-check=0, post-check=0")
+    .add("content-disposition", "attachment; filename=json.json")
+    .add("content-encoding", "gzip")
+    .add("content-type", "application/json;charset=utf-8")
+    .add("date", "Fri, 20 Sep 2019 14:12:06 GMT")
+    .add("expires", "Tue, 31 Mar 1981 05:00:00 GMT")
+    .add("last-modified", "Fri, 20 Sep 2019 14:12:05 GMT")
+    .add("pragma", "no-cache")
+    .add("server", "tsa_o")
+    .add("status", "200, 200 OK")
+    .add("strict-transport-security", "max-age=631138519")
+    .add("x-access-level", "read-write-directmessages")
+    .add("x-client-event-enabled", "true")
+    .add("x-connection-hash", "0bce97311cd0d4e0070842966f5bd9ab")
+    .add("x-content-type-options", "nosniff")
+    .add("x-frame-options", "SAMEORIGIN")
+    .add("x-response-time", "116")
+    .add("x-transaction", "00977e9800e16948")
+    .add("x-tsa-request-body-time", "0")
+    .add("x-twitter-response-tags", "BouncerCompliant")
+    .add("x-xss-protection", "0")
+    .add("Content-Length", "100")
+
   @Benchmark
   def create(): HeaderMap = newMap()
 
   @Benchmark
-  def get(): Option[String] = map.get("Content-Length")
+  def getRandom(): Option[String] = randommap.get("Content-Length")
+
+  @Benchmark
+  def getRealistic(): Option[String] = nonrandommap.get("Content-Length")
 
   @Benchmark
   def createAndAdd(): HeaderMap = newMap().add("Content-Length", "100")
 
   @Benchmark
-  def iterate(b: Blackhole): Unit = map.foreach(h => b.consume(h))
+  @Warmup(iterations = 15)
+  def createAndAdd24(): HeaderMap = {
+    newMap()
+    .add("access-control-allow-credentials", "true")
+    .add("access-control-allow-origin", "https://twitter.com")
+    .add("cache-control", "no-cache, no-store, must-revalidate, pre-check=0, post-check=0")
+    .add("content-disposition", "attachment; filename=json.json")
+    .add("content-encoding", "gzip")
+    .add("content-type", "application/json;charset=utf-8")
+    .add("date", "Fri, 20 Sep 2019 14:12:06 GMT")
+    .add("expires", "Tue, 31 Mar 1981 05:00:00 GMT")
+    .add("last-modified", "Fri, 20 Sep 2019 14:12:05 GMT")
+    .add("pragma", "no-cache")
+    .add("server", "tsa_o")
+    .add("status", "200, 200 OK")
+    .add("strict-transport-security", "max-age=631138519")
+    .add("x-access-level", "read-write-directmessages")
+    .add("x-client-event-enabled", "true")
+    .add("x-connection-hash", "0bce97311cd0d4e0070842966f5bd9ab")
+    .add("x-content-type-options", "nosniff")
+    .add("x-frame-options", "SAMEORIGIN")
+    .add("x-response-time", "116")
+    .add("x-transaction", "00977e9800e16948")
+    .add("x-tsa-request-body-time", "0")
+    .add("x-twitter-response-tags", "BouncerCompliant")
+    .add("x-xss-protection", "0")
+    .add("Content-Length", "100")
+  }
+
+  @Benchmark
+  def iterate(b: Blackhole): Unit = randommap.foreach(h => b.consume(h))
 
   @Benchmark
-  def keySet(): scala.collection.Set[String] = map.keySet
+  def keySet(): scala.collection.Set[String] = randommap.keySet
 
   @Benchmark
-  def iterateKeys(b: Blackhole): Unit = doIterateKeys(b, map)
+  def iterateKeys(b: Blackhole): Unit = doIterateKeys(b, randommap)
 
   @Benchmark
   def iterateRepeatedKeys(b: Blackhole): Unit = doIterateKeys(b, duplicateKeyMap)
 
-  private def doIterateKeys(b: Blackhole, map: HeaderMap): Unit =
-    map.keysIterator.foreach(k => b.consume(k))
-
   @Benchmark
   def iterateValues(b: Blackhole): Unit =
-    map.valuesIterator.foreach(k => b.consume(k))
+    randommap.valuesIterator.foreach(k => b.consume(k))
+
+  private def doIterateKeys(b: Blackhole, map: HeaderMap): Unit =
+    map.keysIterator.foreach(k => b.consume(k))
 }
 
-class DefaultHeaderMapBenchmark extends HeaderMapBenchmark {
-  protected def newMap(): HeaderMap = HeaderMap()
+class HashBackedHeaderMapBenchmark extends HeaderMapBenchmark {
+  protected def newMap(): HeaderMap = HashBackedHeaderMap()
 }
+
+class JTMapBackedHeaderMapBenchmark extends HeaderMapBenchmark {
+  protected def newMap(): HeaderMap = new JTreeMapBackedHeaderMap()
+}