From 2392fb02c3259d7f0b41ff410641accd818bc5d4 Mon Sep 17 00:00:00 2001 From: Ivan Topolnjak Date: Mon, 25 Mar 2019 16:29:21 +0100 Subject: Introduce a TagSet builder and use UnifiedMap from Eclipse Collections as the underlying storage for TagSets. --- build.sbt | 12 +- .../kamon/bench/TagSetCreationBenchmark.scala | 42 ++++ .../scala/kamon/bench/TagSetLookupBenchmark.scala | 55 +++++ kamon-core/src/main/scala/kamon/tag/Lookups.scala | 30 +-- kamon-core/src/main/scala/kamon/tag/TagSet.scala | 245 +++++++++++++++------ 5 files changed, 291 insertions(+), 93 deletions(-) create mode 100644 kamon-core-bench/src/main/scala/kamon/bench/TagSetCreationBenchmark.scala create mode 100644 kamon-core-bench/src/main/scala/kamon/bench/TagSetLookupBenchmark.scala diff --git a/build.sbt b/build.sbt index 5f62b887..538b3804 100644 --- a/build.sbt +++ b/build.sbt @@ -47,6 +47,7 @@ val commonSettings = Seq( ShadeRule.rename("com.grack.nanojson.**" -> "kamon.lib.@0").inAll, ShadeRule.rename("org.jctools.**" -> "kamon.lib.@0").inAll, ShadeRule.rename("fi.iki.elonen.**" -> "kamon.lib.@0").inAll, + ShadeRule.rename("org.eclipse.**" -> "kamon.lib.@0").inAll, ), assemblyExcludedJars in assembly := { val cp = (fullClasspath in assembly).value @@ -56,7 +57,7 @@ val commonSettings = Seq( packageBin in Compile := assembly.value, assemblyJarName in assembly := s"${moduleName.value}_${scalaBinaryVersion.value}-${version.value}.jar", pomPostProcess := { originalPom => { - val shadedGroups = Seq("org.hdrhistogram", "org.jctools", "org.nanohttpd", "com.grack") + val shadedGroups = Seq("org.hdrhistogram", "org.jctools", "org.nanohttpd", "com.grack", "org.eclipse.collections") val filterShadedDependencies = new RuleTransformer(new RewriteRule { override def transform(n: Node): Seq[Node] = { if(n.label == "dependency") { @@ -79,10 +80,11 @@ lazy val core = (project in file("kamon-core")) buildInfoKeys := Seq[BuildInfoKey](version), buildInfoPackage := "kamon.status", libraryDependencies ++= Seq( - "com.typesafe" % "config" % "1.3.1", - "org.hdrhistogram" % "HdrHistogram" % "2.1.9", - "org.jctools" % "jctools-core" % "2.1.1", - "org.slf4j" % "slf4j-api" % "1.7.25" + "com.typesafe" % "config" % "1.3.1", + "org.hdrhistogram" % "HdrHistogram" % "2.1.9", + "org.jctools" % "jctools-core" % "2.1.1", + "org.eclipse.collections" % "eclipse-collections" % "9.2.0", + "org.slf4j" % "slf4j-api" % "1.7.25" ) ) diff --git a/kamon-core-bench/src/main/scala/kamon/bench/TagSetCreationBenchmark.scala b/kamon-core-bench/src/main/scala/kamon/bench/TagSetCreationBenchmark.scala new file mode 100644 index 00000000..9b8f1d7a --- /dev/null +++ b/kamon-core-bench/src/main/scala/kamon/bench/TagSetCreationBenchmark.scala @@ -0,0 +1,42 @@ +package kamon.bench + +import java.util.concurrent.TimeUnit + +import kamon.tag.TagSet +import org.openjdk.jmh.annotations._ + +@BenchmarkMode(Array(Mode.AverageTime)) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(1) +@State(Scope.Benchmark) +class TagSetCreationBenchmark { + + @Param(Array("1", "2", "3", "4", "5", "6")) + var tagCount: Int = 1 + + @Benchmark + def createTagSetFromIndividualKeys(): TagSet = { + var tags = TagSet.Empty + tags = tags.withTag("http.method", "POST") + if(tagCount > 1) tags = tags.withTag("http.url", "http://localhost:8080/test") + if(tagCount > 2) tags = tags.withTag("http.status_code", 200L) + if(tagCount > 3) tags = tags.withTag("error", false) + if(tagCount > 4) tags = tags.withTag("userID", "abcdef") + if(tagCount > 5) tags = tags.withTag("correlationID", "0123456") + + tags + } + + @Benchmark + def createTagSetFromBuilder(): TagSet = { + val tags = TagSet.builder() + tags.add("http.method", "POST") + if(tagCount > 1) tags.add("http.url", "http://localhost:8080/test") + if(tagCount > 2) tags.add("http.status_code", 200L) + if(tagCount > 3) tags.add("error", false) + if(tagCount > 4) tags.add("userID", "abcdef") + if(tagCount > 5) tags.add("correlationID", "0123456") + + tags.create() + } +} diff --git a/kamon-core-bench/src/main/scala/kamon/bench/TagSetLookupBenchmark.scala b/kamon-core-bench/src/main/scala/kamon/bench/TagSetLookupBenchmark.scala new file mode 100644 index 00000000..b8a63d84 --- /dev/null +++ b/kamon-core-bench/src/main/scala/kamon/bench/TagSetLookupBenchmark.scala @@ -0,0 +1,55 @@ +package kamon.bench + +import java.util.concurrent.TimeUnit + +import kamon.tag.TagSet +import org.openjdk.jmh.annotations._ +import kamon.tag.Lookups.any + +@BenchmarkMode(Array(Mode.AverageTime)) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(1) +@State(Scope.Benchmark) +class TagSetLookupBenchmark { + + def builderTags() = TagSet.builder() + .add("http.url", "http://localhost:8080/test") + .add("http.status_code", 200L) + .add("error", false) + .add("userID", "abcdef") + .add("correlationID", "0123456") + .create() + + def keyByKeyTags() = TagSet.Empty + .withTag("http.url", "http://localhost:8080/test") + .withTag("http.status_code", 200L) + .withTag("error", false) + .withTag("userID", "abcdef") + .withTag("correlationID", "0123456") + + + val builderLeft = builderTags() + val builderRight = builderTags() + val keyByKeyLeft = keyByKeyTags() + val keyByKeyRight = keyByKeyTags() + + @Benchmark + def equalityOnBuilderTagSets(): Boolean = { + builderLeft == builderRight + } + + @Benchmark + def equalityOnKeyByKeyTagSets(): Boolean = { + keyByKeyLeft == keyByKeyRight + } + + @Benchmark + def anyLookupOnBuilderTagSet(): Any = { + builderLeft.get(any("userID")) + } + + @Benchmark + def anyLookupOnKeyByKeyTagSet(): Any = { + keyByKeyLeft.get(any("userID")) + } +} diff --git a/kamon-core/src/main/scala/kamon/tag/Lookups.scala b/kamon-core/src/main/scala/kamon/tag/Lookups.scala index beb1996a..9390f472 100644 --- a/kamon-core/src/main/scala/kamon/tag/Lookups.scala +++ b/kamon-core/src/main/scala/kamon/tag/Lookups.scala @@ -13,7 +13,7 @@ object Lookups { * Finds a value associated to the provided key and returns it. If the key is not present then a null is returned. */ def any(key: JString) = new Lookup[Any] { - override def execute(storage: Map[JString, Any]): Any = + override def execute(storage: TagSet.Storage): Any = findAndTransform(key, storage, _any, null) } @@ -23,7 +23,7 @@ object Lookups { * associated with they is not a String then a null is returned. */ def plain(key: JString) = new Lookup[JString] { - override def execute(storage: Map[JString, Any]): JString = + override def execute(storage: TagSet.Storage): JString = findAndTransform(key, storage, _plainString, null) } @@ -33,7 +33,7 @@ object Lookups { * not present or the value associated with they is not a String then a None is returned. */ def option(key: JString) = new Lookup[Option[JString]] { - override def execute(storage: Map[JString, Any]): Option[JString] = + override def execute(storage: TagSet.Storage): Option[JString] = findAndTransform(key, storage, _stringOption, None) } @@ -43,7 +43,7 @@ object Lookups { * is not present or the value associated with they is not a String then Optional.empty() is returned. */ def optional(key: JString) = new Lookup[Optional[String]] { - override def execute(storage: Map[String, Any]): Optional[String] = + override def execute(storage: TagSet.Storage): Optional[String] = findAndTransform(key, storage, _stringOptional, Optional.empty()) } @@ -56,8 +56,8 @@ object Lookups { * This lookup type is guaranteed to return a non-null String representation of value. */ def coerce(key: String) = new Lookup[String] { - override def execute(storage: Map[String, Any]): String = { - val value = storage(key) + override def execute(storage: TagSet.Storage): String = { + val value = storage.get(key) if(value == null) "unknown" else @@ -71,7 +71,7 @@ object Lookups { * associated with they is not a Boolean then a null is returned. */ def plainBoolean(key: String) = new Lookup[JBoolean] { - override def execute(storage: Map[String, Any]): JBoolean = + override def execute(storage: TagSet.Storage): JBoolean = findAndTransform(key, storage, _plainBoolean, null) } @@ -81,7 +81,7 @@ object Lookups { * is not present or the value associated with they is not a Boolean then a None is returned. */ def booleanOption(key: String) = new Lookup[Option[JBoolean]] { - override def execute(storage: Map[String, Any]): Option[JBoolean] = + override def execute(storage: TagSet.Storage): Option[JBoolean] = findAndTransform(key, storage, _booleanOption, None) } @@ -91,7 +91,7 @@ object Lookups { * is not present or the value associated with they is not a Boolean then Optional.empty() is returned. */ def booleanOptional(key: String) = new Lookup[Optional[JBoolean]] { - override def execute(storage: Map[String, Any]): Optional[JBoolean] = + override def execute(storage: TagSet.Storage): Optional[JBoolean] = findAndTransform(key, storage, _booleanOptional, Optional.empty()) } @@ -101,7 +101,7 @@ object Lookups { * associated with they is not a Long then a null is returned. */ def plainLong(key: String) = new Lookup[JLong] { - override def execute(storage: Map[String, Any]): JLong = + override def execute(storage: TagSet.Storage): JLong = findAndTransform(key, storage, _plainLong, null) } @@ -111,7 +111,7 @@ object Lookups { * not present or the value associated with they is not a Long then a None is returned. */ def longOption(key: String) = new Lookup[Option[JLong]] { - override def execute(storage: Map[String, Any]): Option[JLong] = + override def execute(storage: TagSet.Storage): Option[JLong] = findAndTransform(key, storage, _longOption, None) } @@ -121,7 +121,7 @@ object Lookups { * is not present or the value associated with they is not a Long then Optional.empty() is returned. */ def longOptional(key: String) = new Lookup[Optional[JLong]] { - override def execute(storage: Map[String, Any]): Optional[JLong] = + override def execute(storage: TagSet.Storage): Optional[JLong] = findAndTransform(key, storage, _longOptional, Optional.empty()) } @@ -129,13 +129,13 @@ object Lookups { //////////////////////////////////////////////////////////////// // Transformation helpers for the lookup DSL // //////////////////////////////////////////////////////////////// - - private def findAndTransform[T, R](key: String, storage: Map[String, Any], transform: R => T, default: T) + @inline + private def findAndTransform[T, R](key: String, storage: TagSet.Storage, transform: R => T, default: T) (implicit ct: ClassTag[R]): T = { // This assumes that this code will only be used to lookup values from a Tags instance // for which the underlying map always has "null" as the default value. - val value = storage(key) + val value = storage.get(key) if(value == null || !ct.runtimeClass.isInstance(value)) default diff --git a/kamon-core/src/main/scala/kamon/tag/TagSet.scala b/kamon-core/src/main/scala/kamon/tag/TagSet.scala index 1090ca5a..c304a9df 100644 --- a/kamon-core/src/main/scala/kamon/tag/TagSet.scala +++ b/kamon-core/src/main/scala/kamon/tag/TagSet.scala @@ -2,9 +2,10 @@ package kamon.tag import kamon.tag.TagSet.Lookup -import scala.collection.JavaConverters.asScalaIteratorConverter import java.lang.{Boolean => JBoolean, Long => JLong, String => JString} +import java.util.function.BiConsumer +import org.eclipse.collections.impl.map.mutable.UnifiedMap import org.slf4j.LoggerFactory /** @@ -34,9 +35,10 @@ import org.slf4j.LoggerFactory * cumbersome operation is rarely necessary on user-facing code. * */ -class TagSet private(private val _tags: Map[String, Any]) { +class TagSet private(private val _underlying: UnifiedMap[String, Any]) { import TagSet.withPair + /** * Creates a new TagSet instance that includes the provided key/value pair. If the provided key was already associated * with another value then the previous value will be discarded and overwritten with the provided one. @@ -67,7 +69,7 @@ class TagSet private(private val _tags: Map[String, Any]) { * overwritten with the provided one. */ def withTags(other: TagSet): TagSet = - new TagSet(_tags ++ other._tags) + and(other) /** @@ -99,22 +101,26 @@ class TagSet private(private val _tags: Map[String, Any]) { * instance are associated to a key present on the provided instance then the previous value will be discarded and * overwritten with the provided one. */ - def and(other: TagSet): TagSet = - new TagSet(_tags ++ other._tags) + def and(other: TagSet): TagSet = { + val mergedMap = new UnifiedMap[String, Any](other._underlying.size() + this._underlying.size()) + mergedMap.putAll(this._underlying) + mergedMap.putAll(other._underlying) + new TagSet(mergedMap) + } /** * Returns whether this TagSet instance does not contain any tags. */ def isEmpty(): Boolean = - _tags.isEmpty + _underlying.isEmpty /** * Returns whether this TagSet instance contains any tags. */ def nonEmpty(): Boolean = - _tags.nonEmpty + !_underlying.isEmpty /** @@ -122,7 +128,7 @@ class TagSet private(private val _tags: Map[String, Any]) { * built-in lookups on the [[Lookups]] companion object for more information. */ def get[T](lookup: Lookup[T]): T = - lookup.execute(_tags) + lookup.execute(_storage) /** @@ -132,14 +138,19 @@ class TagSet private(private val _tags: Map[String, Any]) { * * The returned sequence contains immutable values and is safe to share across threads. */ - def all(): Seq[Tag] = - _tags.foldLeft(List.empty[Tag]) { - case (ts, (key, value)) => value match { - case v: String => new immutable.String(key, v) :: ts - case v: Boolean => new immutable.Boolean(key, v) :: ts - case v: Long => new immutable.Long(key, v) :: ts + def all(): Seq[Tag] = { + var tags: List[Tag] = Nil + + _underlying.forEach(new BiConsumer[String, Any] { + override def accept(key: String, value: Any): Unit = value match { + case v: String => tags = new TagSet.immutable.String(key, v) :: tags + case v: Boolean => tags = new TagSet.immutable.Boolean(key, v) :: tags + case v: Long => tags = new TagSet.immutable.Long(key, v) :: tags } - } + }) + + tags + } /** @@ -149,59 +160,62 @@ class TagSet private(private val _tags: Map[String, Any]) { * structure that will be sent to the external systems. */ def iterator(): Iterator[Tag] = new Iterator[Tag] { - private val _entriesIterator = _tags.iterator - private var _longTag: mutable.Long = null - private var _stringTag: mutable.String = null - private var _booleanTag: mutable.Boolean = null + private val _entriesIterator = _underlying.keyValuesView().iterator() + private var _longTag: TagSet.mutable.Long = null + private var _stringTag: TagSet.mutable.String = null + private var _booleanTag: TagSet.mutable.Boolean = null override def hasNext: Boolean = _entriesIterator.hasNext override def next(): Tag = { - val (key, value) = _entriesIterator.next() - value match { - case v: String => stringTag(key, v) - case v: Boolean => booleanTag(key, v) - case v: Long => longTag(key, v) + val pair = _entriesIterator.next() + pair.getTwo match { + case v: String => stringTag(pair.getOne, v) + case v: Boolean => booleanTag(pair.getOne, v) + case v: Long => longTag(pair.getOne, v) } } private def stringTag(key: JString, value: JString): Tag.String = if(_stringTag == null) { - _stringTag = new mutable.String(key, value) + _stringTag = new TagSet.mutable.String(key, value) _stringTag } else _stringTag.updated(key, value) private def booleanTag(key: JString, value: JBoolean): Tag.Boolean = if(_booleanTag == null) { - _booleanTag = new mutable.Boolean(key, value) + _booleanTag = new TagSet.mutable.Boolean(key, value) _booleanTag } else _booleanTag.updated(key, value) private def longTag(key: JString, value: JLong): Tag.Long = if(_longTag == null) { - _longTag = new mutable.Long(key, value) + _longTag = new TagSet.mutable.Long(key, value) _longTag } else _longTag.updated(key, value) } - override def equals(other: Any): Boolean = - other != null && other.isInstanceOf[TagSet] && other.asInstanceOf[TagSet]._tags == this._tags + other != null && other.isInstanceOf[TagSet] && other.asInstanceOf[TagSet]._underlying == _underlying + override def hashCode(): Int = + _underlying.hashCode() override def toString: JString = { val sb = new StringBuilder() sb.append("Tags{") var hasTags = false - _tags.foreach { case (k, v) => + val iterator = _underlying.keyValuesView().iterator() + while(iterator.hasNext) { + val pair = iterator.next() if(hasTags) sb.append(",") - sb.append(k) + sb.append(pair.getOne) .append("=") - .append(v) + .append(pair.getTwo) hasTags = true } @@ -209,36 +223,104 @@ class TagSet private(private val _tags: Map[String, Any]) { sb.append("}").toString() } - private object immutable { - case class String(key: JString, value: JString) extends Tag.String - case class Boolean(key: JString, value: JBoolean) extends Tag.Boolean - case class Long(key: JString, value: JLong) extends Tag.Long + private val _storage = new TagSet.Storage { + override def get(key: String): Any = _underlying.get(key) + override def iterator(): Iterator[Tag] = TagSet.this.iterator() + override def isEmpty(): Boolean = _underlying.isEmpty } +} - private object mutable { - case class String(var key: JString, var value: JString) extends Tag.String with Updateable[JString] - case class Boolean(var key: JString, var value: JBoolean) extends Tag.Boolean with Updateable[JBoolean] - case class Long(var key: JString, var value: JLong) extends Tag.Long with Updateable[JLong] +object TagSet { - trait Updateable[T] { - var key: JString - var value: T + /** + * Describes a strategy to lookup values from a TagSet instance. Implementations of this interface will be provided + * with the actual data structure containing the tags and must perform any necessary runtime type checks to ensure + * that the returned value is in assignable to the expected type T. + * + * Several implementation are provided in the Lookup companion object and it is recommended to import and use those + * definitions when looking up keys from a Tags instance. + */ + trait Lookup[T] { + + /** + * Tries to find a value on a TagSet.Storage and returns a representation of it. In some cases the stored object + * might be returned as-is, in some others it might be transformed or wrapped on Option/Optional to handle missing + * values. Take a look at the Lookups companion object for examples.. + */ + def execute(storage: TagSet.Storage): T + } + + + /** + * A temporary structure that accumulates key/value and creates a new TagSet instance from them. It is faster to use + * a Builder and add tags to it rather than creating TagSet and add each key individually. Builder instances rely on + * internal mutable state and are not thread safe. + */ + trait Builder { + + /** Adds a new key/value pair to the builder. */ + def add(key: String, value: Any): Builder + + /** Creates a new TagSet instance that includes all valid key/value pairs added to this builder. */ + def create(): TagSet + } + + + /** + * Abstracts the actual storage used for a TagSet. This interface resembles a stripped down interface of an immutable + * map of String to Any, used to expose the underlying structure where tags are stored to Lookups, without leaking + * the actual implementation. + */ + trait Storage { + + /** + * Gets the value associated with the provided key, or null if no value was found. The decision of returning null + * when the key is not present is a conscious one, backed by the fact that users will never be exposed to this + * storage interface and they can decide their way of handling missing values by selecting an appropriate lookup. + */ + def get(key: String): Any + + /** + * Provides an Iterator that can go through all key/value pairs contained in the Storage instance. + */ + def iterator(): Iterator[Tag] + + /** + * Returns true if there are no tags in the storage. + */ + def isEmpty(): Boolean - def updated(key: JString, value: T): this.type = { - this.key = key - this.value = value - this - } - } } -} -object TagSet { /** * A valid instance of tags that doesn't contain any pairs. */ - val Empty = new TagSet(Map.empty.withDefaultValue(null)) + val Empty = new TagSet(UnifiedMap.newMap[String, Any]()) + + + /** + * Creates a new Builder instance. + */ + def builder(): Builder = new Builder { + private var _tagCount = 0 + private var _tags: List[(String, Any)] = Nil + + override def add(key: String, value: Any): Builder = { + if(isValidPair(key, value)) { + _tagCount += 1 + _tags = (key -> value) :: _tags + } + + this + } + + override def create(): TagSet = { + val newMap = new UnifiedMap[String, Any](_tagCount) + _tags.foreach { case (key, value) => newMap.put(key, value) } + new TagSet(newMap) + } + } /** @@ -266,8 +348,12 @@ object TagSet { * Constructs a new TagSet instance from a Map. The returned TagSet will only contain the entries that have String, * Long or Boolean values from the supplied map, any other entry in the map will be ignored. */ - def from(map: Map[String, Any]): TagSet = - new TagSet(map.filter { case (k, v) => isValidPair(k, v) } withDefaultValue(null)) + def from(map: Map[String, Any]): TagSet = { + val unifiedMap = new UnifiedMap[String, Any](map.size) + map.foreach { pair => if(isValidPair(pair._1, pair._2)) unifiedMap.put(pair._1, pair._2)} + + new TagSet(unifiedMap) + } /** @@ -275,22 +361,25 @@ object TagSet { * Long or Boolean values from the supplied map, any other entry in the map will be ignored. */ def from(map: java.util.Map[String, Any]): TagSet = { - val allowedTags = Map.newBuilder[String, Any] - map.entrySet() - .iterator() - .asScala - .foreach(e => if(isValidPair(e.getKey, e.getValue)) allowedTags += (e.getKey -> e.getValue)) + val unifiedMap = new UnifiedMap[String, Any](map.size) + map.forEach(new BiConsumer[String, Any] { + override def accept(key: String, value: Any): Unit = + if(isValidPair(key, value)) unifiedMap.put(key, value) + }) - new TagSet(allowedTags.result().withDefaultValue(null)) + new TagSet(unifiedMap) } private val _logger = LoggerFactory.getLogger(classOf[TagSet]) private def withPair(parent: TagSet, key: String, value: Any): TagSet = - if(isValidPair(key, value)) - new TagSet(parent._tags.updated(key, value)) - else + if(isValidPair(key, value)) { + val mergedMap = new UnifiedMap[String, Any](parent._underlying.size() + 1) + mergedMap.putAll(parent._underlying) + mergedMap.put(key, value) + new TagSet(mergedMap) + } else parent private def isValidPair(key: String, value: Any): Boolean = { @@ -313,16 +402,26 @@ object TagSet { private def isAllowedTagValue(v: Any): Boolean = v != null && (v.isInstanceOf[String] || v.isInstanceOf[Boolean] || v.isInstanceOf[Long]) + private object immutable { + case class String(key: JString, value: JString) extends Tag.String + case class Boolean(key: JString, value: JBoolean) extends Tag.Boolean + case class Long(key: JString, value: JLong) extends Tag.Long + } - /** - * Describes a strategy to lookup values from a TagSet instance. Implementations of this interface will be provided - * with the actual data structure containing the tags and must perform any necessary runtime type checks to ensure - * that the returned value is in assignable to the expected type T. - * - * Several implementation are provided in the Lookup companion object and it is recommended to import and use those - * definitions when looking up keys from a Tags instance. - */ - trait Lookup[T] { - def execute(storage: Map[String, Any]): T + private object mutable { + case class String(var key: JString, var value: JString) extends Tag.String with Updateable[JString] + case class Boolean(var key: JString, var value: JBoolean) extends Tag.Boolean with Updateable[JBoolean] + case class Long(var key: JString, var value: JLong) extends Tag.Long with Updateable[JLong] + + trait Updateable[T] { + var key: JString + var value: T + + def updated(key: JString, value: T): this.type = { + this.key = key + this.value = value + this + } + } } } \ No newline at end of file -- cgit v1.2.3