Stop expensive conversions from scala to java lists
Java implementation of Gson does not know how to serialize scala
collections out of the box. For this reason CommitsStatistics
and CommitInfo objects had been modeled with java.util.List and
java.util.Set instead, just to help gson to serialize them.
This change avoids unneded conversion from java to scala and vice-versa
by:
- Remodeling of CommitsStatistics and CommitInfo to use scala native
collections.
- Registering a scala iterable serializer to Gson so that it knows how
to serialize scala lists and sets.
This bumps up the latency of the contributors endpoint of around 50%.
For example, when extracting the entire history of master branch for
Gerrit repo, latency decreases from 58s to 27s.
Change-Id: I1ba7e81eb81ebcffaa431a4340fb9da09e6a5976
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
index a62979b..8dbd4d4 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
@@ -165,7 +165,7 @@
ManagedResource.use(repoManager.openRepository(projectRes.getNameKey)) { repo =>
- val stats = new Statistics(repo, new BotLikeExtractorImpl(botLikeIdentifiers), commentLinks.asJava)
+ val stats = new Statistics(repo, new BotLikeExtractorImpl(botLikeIdentifiers), commentLinks)
val branchesExtractor = extractBranches.option(new BranchesExtractor(repo))
histogram.get(repo, new AggregatedHistogramFilterByDates(startDate, stopDate, branchesExtractor, aggregationStrategy))
@@ -176,7 +176,7 @@
}
}
-case class CommitInfo(sha1: String, date: Long, merge: Boolean, botLike: Boolean, files: java.util.Set[String])
+case class CommitInfo(sha1: String, date: Long, merge: Boolean, botLike: Boolean, files: Set[String])
case class IssueInfo(code: String, link: String)
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala
index e5f9723..ee230a3 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala
@@ -24,7 +24,7 @@
import org.eclipse.jgit.util.io.DisabledOutputStream
import org.slf4j.LoggerFactory
-import scala.collection.JavaConversions._
+import scala.collection.JavaConverters._
import scala.util.matching.Regex
/**
@@ -82,7 +82,7 @@
val EmptyBotMerge = EmptyMerge.copy(isForBotLike = true)
}
-class Statistics(repo: Repository, botLikeExtractor: BotLikeExtractor, commentInfoList: java.util.List[CommentLinkInfo] = Nil) {
+class Statistics(repo: Repository, botLikeExtractor: BotLikeExtractor, commentInfoList: List[CommentLinkInfo] = Nil) {
val log = LoggerFactory.getLogger(classOf[Statistics])
val replacers = commentInfoList.map(info =>
@@ -142,13 +142,13 @@
df.setRepository(repo)
df.setDiffComparator(RawTextComparator.DEFAULT)
df.setDetectRenames(true)
- val diffs = df.scan(oldTree, newTree)
+ val diffs = df.scan(oldTree, newTree).asScala
case class Lines(deleted: Int, added: Int) {
def +(other: Lines) = Lines(deleted + other.deleted, added + other.added)
}
val lines = (for {
diff <- diffs
- edit <- df.toFileHeader(diff).toEditList
+ edit <- df.toFileHeader(diff).toEditList.asScala
} yield Lines(edit.getEndA - edit.getBeginA, edit.getEndB - edit.getBeginB)).fold(Lines(0, 0))(_ + _)
val files: Set[String] = diffs.map(df.toFileHeader(_).getNewPath).toSet
@@ -159,7 +159,7 @@
}
}
- def extractIssues(commitMessage: String): List[IssueInfo] = {
+ def extractIssues(commitMessage: String): List[IssueInfo] =
replacers.flatMap {
case Replacer(pattern, replaced) =>
pattern.findAllIn(commitMessage)
@@ -167,8 +167,7 @@
val transformed = pattern.replaceAllIn(code, replaced)
IssueInfo(code, transformed)
})
- }.toList
- }
+ }
case class Replacer(pattern: Regex, replaced: String)
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala
index 58b5dc5..cca743d 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala
@@ -17,20 +17,32 @@
import java.io.PrintWriter
import com.google.gerrit.server.OutputFormat
-import com.google.gson.{Gson, GsonBuilder}
+import com.google.gson.{Gson, GsonBuilder, JsonSerializer}
import com.google.inject.Singleton
-import com.googlesource.gerrit.plugins.analytics.CommitInfo
+import java.lang.reflect.Type
+
+import com.google.gson._
@Singleton
class GsonFormatter {
val gsonBuilder: GsonBuilder =
OutputFormat.JSON_COMPACT.newGsonBuilder
+ .registerTypeHierarchyAdapter(classOf[Iterable[Any]], new IterableSerializer)
def format[T](values: TraversableOnce[T], out: PrintWriter) {
val gson: Gson = gsonBuilder.create
+
for (value <- values) {
gson.toJson(value, out)
out.println()
}
}
+
+ class IterableSerializer extends JsonSerializer[Iterable[Any]] {
+ override def serialize(src: Iterable[Any], typeOfSrc: Type, context: JsonSerializationContext): JsonElement = {
+ import scala.collection.JavaConverters._
+ context.serialize(src.asJava)
+ }
+ }
+
}
\ No newline at end of file
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitInfoSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitInfoSpec.scala
index c8aff83..4873328 100644
--- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitInfoSpec.scala
+++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitInfoSpec.scala
@@ -1,16 +1,15 @@
package com.googlesource.gerrit.plugins.analytics.test
-import com.google.common.collect.Sets.newHashSet
-import com.google.gerrit.server.OutputFormat
import com.googlesource.gerrit.plugins.analytics.CommitInfo
+import com.googlesource.gerrit.plugins.analytics.common.GsonFormatter
import org.scalatest.{FlatSpec, Matchers}
class CommitInfoSpec extends FlatSpec with Matchers {
"CommitInfo" should "be serialised as JSON correctly" in {
- val commitInfo = CommitInfo(sha1 = "sha", date = 1000l, merge = false, botLike = false, files = newHashSet("file1", "file2"))
+ val commitInfo = CommitInfo(sha1 = "sha", date = 1000l, merge = false, botLike = false, files = Set("file1", "file2"))
- val gsonBuilder = OutputFormat.JSON_COMPACT.newGsonBuilder
+ val gsonBuilder = new GsonFormatter().gsonBuilder
val actual = gsonBuilder.create().toJson(commitInfo)
List(actual) should contain oneOf(
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala
index c9031c6..2900e69 100644
--- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala
+++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala
@@ -20,8 +20,6 @@
import org.eclipse.jgit.internal.storage.file.FileRepository
import org.scalatest.{FlatSpec, Inside, Matchers}
-import scala.collection.JavaConverters._
-
class CommitStatisticsCommentLinkSpec extends FlatSpec with GitTestCase with Matchers with Inside {
def createCommentLinkInfo(pattern: String, link: Option[String] = None, html: Option[String] = None) = {
@@ -33,11 +31,11 @@
}
class TestEnvironment(val repo: FileRepository = new FileRepository(testRepo),
- val commentLinks: java.util.List[CommentLinkInfo] = Seq(
+ val commentLinks: List[CommentLinkInfo] = List(
createCommentLinkInfo(pattern = "(bug\\s+#?)(\\d+)",
link = Some("http://bugs.example.com/show_bug.cgi?id=$2")),
createCommentLinkInfo(pattern = "([Bb]ug:\\s+)(\\d+)",
- html = Some("$1<a href=\"http://trak.example.com/$2\">$2</a>"))).asJava) {
+ html = Some("$1<a href=\"http://trak.example.com/$2\">$2</a>")))) {
lazy val stats = new Statistics(repo, TestBotLikeExtractor, commentLinks)
}
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala
index d5257ef..332c724 100644
--- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala
+++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala
@@ -14,7 +14,6 @@
package com.googlesource.gerrit.plugins.analytics.test
-import com.google.common.collect.Sets.newHashSet
import com.googlesource.gerrit.plugins.analytics.CommitInfo
import com.googlesource.gerrit.plugins.analytics.common.{CommitsStatistics, Statistics}
import org.eclipse.jgit.internal.storage.file.FileRepository
@@ -39,10 +38,10 @@
}
it should "sum to another compatible CommitStatistics generating an aggregated stat" in {
- val commit1 = CommitInfo("sha_1", 1000l, false, botLike = false, newHashSet("file1"))
- val commit2 = CommitInfo("sha_2", 2000l, false, botLike = false, newHashSet("file1"))
- val commit3 = CommitInfo("sha_3", 3000l, false, botLike = false, newHashSet("file2"))
- val commit4 = CommitInfo("sha_4", 1000l, false, botLike = false, newHashSet("file1"))
+ val commit1 = CommitInfo("sha_1", 1000l, false, botLike = false, Set("file1"))
+ val commit2 = CommitInfo("sha_2", 2000l, false, botLike = false, Set("file1"))
+ val commit3 = CommitInfo("sha_3", 3000l, false, botLike = false, Set("file2"))
+ val commit4 = CommitInfo("sha_4", 1000l, false, botLike = false, Set("file1"))
val stat1 = CommitsStatistics(3, 4, false, false, List(commit1, commit2))
val stat2 = CommitsStatistics(5, 7, false, false, List(commit3, commit4))
