From 71b87d6a54e0ca32c7670000315e60eeae032878 Mon Sep 17 00:00:00 2001 From: Doug Roper Date: Thu, 14 Feb 2019 18:44:01 -0500 Subject: [PATCH 1/3] Optimize StandaloneAhcWSRequest add* methods. (#301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary ------- `StandaloneAhcWSRequest` stores query params and headers as immutable maps that support structural sharing, yet the implementation of `addQueryStringParameters((String, String)*)` and `addHttpHeaders(hdrs: (String, String)*)` methods rebuild the entire map from scratch on each call. That's an `O(n)` implementation over an `O(eC)` data structure. Inserting *n* elements is *O(n²)* complexity. Benchmarks ---------- I put together a benchmark to add *size* query params, then measure adding the next one. Before ``` [info] Benchmark (size) Mode Cnt Score Error Units [info] AddQueryParamsBench.addParams 1 avgt 10 417.620 ± 17.237 ns/op [info] AddQueryParamsBench.addParams 10 avgt 10 2257.053 ± 76.440 ns/op [info] AddQueryParamsBench.addParams 100 avgt 10 18839.600 ± 801.003 ns/op [info] AddQueryParamsBench.addParams 1000 avgt 10 224491.116 ± 12133.870 ns/op [info] AddQueryParamsBench.addParams 10000 avgt 10 3065837.189 ± 82797.559 ns/op ``` After ``` [info] Benchmark (size) Mode Cnt Score Error Units [info] AddQueryParamsBench.addParams 1 avgt 10 47.978 ± 0.388 ns/op [info] AddQueryParamsBench.addParams 10 avgt 10 76.495 ± 0.663 ns/op [info] AddQueryParamsBench.addParams 100 avgt 10 118.205 ± 1.072 ns/op [info] AddQueryParamsBench.addParams 1000 avgt 10 141.506 ± 1.681 ns/op [info] AddQueryParamsBench.addParams 10000 avgt 10 151.452 ± 2.560 ns/op ``` --- ...StandaloneAhcWSRequestBenchMapsBench.scala | 77 +++++++++++++++++++ build.sbt | 26 ++++++- .../libs/ws/ahc/StandaloneAhcWSRequest.scala | 36 +++++++-- project/plugins.sbt | 4 +- 4 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 bench/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequestBenchMapsBench.scala diff --git a/bench/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequestBenchMapsBench.scala b/bench/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequestBenchMapsBench.scala new file mode 100644 index 00000000..da787afc --- /dev/null +++ b/bench/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequestBenchMapsBench.scala @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2009-2019 Lightbend Inc. + */ + +package play.api.libs.ws.ahc + +import java.util.concurrent.TimeUnit + +import akka.stream.Materializer +import org.openjdk.jmh.annotations._ +import org.openjdk.jmh.infra.Blackhole +import play.api.libs.ws.StandaloneWSRequest + +/** + * Tests Map-backed features of [[StandaloneAhcWSRequest]]. + * + * ==Quick Run from sbt== + * + * > bench/jmh:run .*StandaloneAhcWSRequestBenchMapsBench + * + * ==Using Oracle Flight Recorder== + * + * To record a Flight Recorder file from a JMH run, run it using the jmh.extras.JFR profiler: + * > bench/jmh:run -prof jmh.extras.JFR .*StandaloneAhcWSRequestBenchMapsBench + * + * Compare your results before/after on your machine. Don't trust the ones in scaladoc. + * + * Sample benchmark results: + * {{{ + * > bench/jmh:run .*StandaloneAhcWSRequestBenchMapsBench + * [info] Benchmark (size) Mode Cnt Score Error Units + * [info] StandaloneAhcWSRequestBenchMapsBench.addHeaders 1 avgt 162.673 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addHeaders 10 avgt 195.672 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addHeaders 100 avgt 278.829 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addHeaders 1000 avgt 356.446 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addHeaders 10000 avgt 308.384 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addQueryParams 1 avgt 42.123 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addQueryParams 10 avgt 82.650 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addQueryParams 100 avgt 90.095 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addQueryParams 1000 avgt 123.221 ns/op + * [info] StandaloneAhcWSRequestBenchMapsBench.addQueryParams 10000 avgt 141.556 ns/op + * }}} + * + * @see https://github.com/ktoso/sbt-jmh + */ +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@BenchmarkMode(Array(Mode.AverageTime)) +@Fork(jvmArgsAppend = Array("-Xmx350m", "-XX:+HeapDumpOnOutOfMemoryError"), value = 1) +@State(Scope.Benchmark) +class StandaloneAhcWSRequestBenchMapsBench { + + private implicit val materializer: Materializer = null // we're not actually going to execute anything. + private var exampleRequest: StandaloneWSRequest = _ + + @Param(Array("1", "10", "100", "1000", "10000")) + private var size: Int = _ + + @Setup def setup(): Unit = { + val params = (1 to size) + .map(_.toString) + .map(s => s -> s) + + exampleRequest = StandaloneAhcWSRequest(new StandaloneAhcWSClient(null), "https://www.example.com") + .addQueryStringParameters(params: _*) + .addHttpHeaders(params: _*) + } + + @Benchmark + def addQueryParams(bh: Blackhole): Unit = { + bh.consume(exampleRequest.addQueryStringParameters("nthParam" -> "nthParam")) + } + + @Benchmark + def addHeaders(bh: Blackhole): Unit = { + bh.consume(exampleRequest.addHttpHeaders("nthHeader" -> "nthHeader")) + } +} diff --git a/build.sbt b/build.sbt index 2287395b..53b180aa 100644 --- a/build.sbt +++ b/build.sbt @@ -126,8 +126,7 @@ val disableDocs = Seq[Setting[_]]( val disablePublishing = Seq[Setting[_]]( publishArtifact := false, - skip in publish := true, - crossScalaVersions := Seq(scala212) + skip in publish := true ) lazy val shadeAssemblySettings = commonSettings ++ Seq( @@ -382,7 +381,6 @@ lazy val `integration-tests` = project.in(file("integration-tests")) .settings(disableDocs) .settings(disablePublishing) .settings( - crossScalaVersions := Seq(scala213, scala212, scala211), fork in Test := true, concurrentRestrictions += Tags.limitAll(1), // only one integration test at a time testOptions in Test := Seq(Tests.Argument(TestFrameworks.JUnit, "-a", "-v")), @@ -397,6 +395,24 @@ lazy val `integration-tests` = project.in(file("integration-tests")) ) .disablePlugins(sbtassembly.AssemblyPlugin) +//--------------------------------------------------------------- +// Benchmarks (run manually) +//--------------------------------------------------------------- + +lazy val bench = project + .in(file("bench")) + .enablePlugins(JmhPlugin) + .dependsOn( + `play-ws-standalone`, + `play-ws-standalone-json`, + `play-ws-standalone-xml`, + `play-ahc-ws-standalone` + ) + .settings(commonSettings) + .settings(formattingSettings) + .settings(disableDocs) + .settings(disablePublishing) + //--------------------------------------------------------------- // Root Project //--------------------------------------------------------------- @@ -413,13 +429,15 @@ lazy val root = project .settings(formattingSettings) .settings(disableDocs) .settings(disablePublishing) + .settings(crossScalaVersions := Seq(scala212)) .aggregate( `shaded`, `play-ws-standalone`, `play-ws-standalone-json`, `play-ws-standalone-xml`, `play-ahc-ws-standalone`, - `integration-tests` + `integration-tests`, + bench ) .disablePlugins(sbtassembly.AssemblyPlugin) diff --git a/play-ahc-ws-standalone/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequest.scala b/play-ahc-ws-standalone/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequest.scala index 08239123..3f451e08 100644 --- a/play-ahc-ws-standalone/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequest.scala +++ b/play-ahc-ws-standalone/src/main/scala/play/api/libs/ws/ahc/StandaloneAhcWSRequest.scala @@ -67,12 +67,23 @@ case class StandaloneAhcWSRequest( override def withAuth(username: String, password: String, scheme: WSAuthScheme): Self = copy(auth = Some((username, password, scheme))) + override def addHttpHeaders(hdrs: (String, String)*): StandaloneWSRequest = { + val newHeaders = buildHeaders(headers, hdrs: _*) + copy(headers = newHeaders) + } + override def withHttpHeaders(hdrs: (String, String)*): Self = { - var newHeaders = hdrs.foldLeft(TreeMap[String, Seq[String]]()(CaseInsensitiveOrdered)) { - (m, hdr) => + val emptyMap = TreeMap[String, Seq[String]]()(CaseInsensitiveOrdered) + val newHeaders = buildHeaders(emptyMap, hdrs: _*) + copy(headers = newHeaders) + } + + private def buildHeaders(origHeaders: Map[String, Seq[String]], hdrs: (String, String)*): Map[String, Seq[String]] = { + var newHeaders = hdrs + .foldLeft(origHeaders) { (m, hdr) => if (m.contains(hdr._1)) m.updated(hdr._1, m(hdr._1) :+ hdr._2) else m + (hdr._1 -> Seq(hdr._2)) - } + } // preserve the content type newHeaders = contentType match { @@ -82,13 +93,24 @@ case class StandaloneAhcWSRequest( newHeaders } - copy(headers = newHeaders) + newHeaders + } + + override def addQueryStringParameters(parameters: (String, String)*): StandaloneWSRequest = { + val newQueryString = buildQueryParams(queryString, parameters: _*) + copy(queryString = newQueryString) + } + + override def withQueryStringParameters(parameters: (String, String)*): Self = { + val newQueryString = buildQueryParams(Map.empty[String, Seq[String]], parameters: _*) + copy(queryString = newQueryString) } - override def withQueryStringParameters(parameters: (String, String)*): Self = - copy(queryString = parameters.foldLeft(Map.empty[String, Seq[String]]) { + private def buildQueryParams(orig: Map[String, Seq[String]], params: (String, String)*): Map[String, Seq[String]] = { + params.foldLeft(orig) { case (m, (k, v)) => m + (k -> (v +: m.getOrElse(k, Nil))) - }) + } + } override def withCookies(cookies: WSCookie*): StandaloneWSRequest = copy(cookies = cookies) diff --git a/project/plugins.sbt b/project/plugins.sbt index 9f676635..a46735d7 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -16,4 +16,6 @@ addSbtPlugin("com.typesafe" % "sbt-mima-plugin" % "0.1.18") addSbtPlugin("com.gilt" % "sbt-dependency-graph-sugar" % "0.9.0") -addSbtPlugin("de.heikoseeberger" % "sbt-header" % "5.0.0") \ No newline at end of file +addSbtPlugin("de.heikoseeberger" % "sbt-header" % "5.0.0") + +addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.3.4") From 8ccad707a062f967827c886949309bdcba842563 Mon Sep 17 00:00:00 2001 From: Marcos Pereira Date: Tue, 19 Feb 2019 10:18:36 -0500 Subject: [PATCH 2/3] Add probot settings (#306) Add probot settings configuration. ## References https://github.com/probot/settings --- .github/settings.yml | 102 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 .github/settings.yml diff --git a/.github/settings.yml b/.github/settings.yml new file mode 100644 index 00000000..f6bcff6e --- /dev/null +++ b/.github/settings.yml @@ -0,0 +1,102 @@ +# These settings are synced to GitHub by https://probot.github.io/apps/settings/ +repository: + homepage: "https://www.playframework.com/documentation/latest/JavaWS" + topics: playframework, asynchttpclient, scala, http-client, java, asynchronous, reactive, reactive-streams + private: false + has_issues: true + has_projects: true + # We don't need wiki since the documentation lives in playframework.com + has_wiki: false + has_downloads: true + default_branch: master + allow_squash_merge: true + allow_merge_commit: false + allow_rebase_merge: false + +teams: + - name: core + permission: admin + - name: integrators + permission: write + +branches: + - name: "*" + protection: + required_pull_request_reviews: + required_approving_review_count: 1 + dismiss_stale_reviews: true + # Require status checks to pass before merging + required_status_checks: + # The list of status checks to require in order to merge into this branch + contexts: ["Travis CI - Pull Request", "typesafe-cla-validator"] + +labels: + - color: f9d0c4 + name: "closed:declined" + - color: f9d0c4 + name: "closed:duplicated" + oldname: duplicate + - color: f9d0c4 + name: "closed:invalid" + oldname: invalid + - color: f9d0c4 + name: "closed:question" + oldname: question + - color: f9d0c4 + name: "closed:wontfix" + oldname: wontfix + - color: 7057ff + name: "good first issue" + - color: 7057ff + name: "Hacktoberfest" + - color: 7057ff + name: "help wanted" + - color: cceecc + name: "status:backlog" + oldname: backlog + - color: b60205 + name: "status:block-merge" + oldname: block-merge + - color: b60205 + name: "status:blocked" + - color: 0e8a16 + name: "status:in-progress" + - color: 0e8a16 + name: "status:merge-when-green" + oldname: merge-when-green + - color: fbca04 + name: "status:needs-backport" + oldname: needs-backport + - color: fbca04 + name: "status:needs-forwardport" + - color: fbca04 + name: "status:needs-info" + - color: fbca04 + name: "status:needs-verification" + oldname: needs-confirmation + - color: 0e8a16 + name: "status:ready" + - color: fbca04 + name: "status:to-review" + oldname: review + - color: c5def5 + name: "topic:build/tests" + - color: c5def5 + name: "topic:dev-environment" + - color: c5def5 + name: "topic:documentation" + - color: c5def5 + name: "topic:jdk-next" + - color: b60205 + name: "type:defect" + oldname: bug + - color: 0052cc + name: "type:feature" + - color: 0052cc + name: "type:improvement" + oldname: enhancement + - color: 0052cc + name: "type:updates" + - color: 0052cc + name: "type:upstream" + oldname: akka-http-backend From b002107bba48e39b393216f5a3e668c734d17401 Mon Sep 17 00:00:00 2001 From: sullis Date: Thu, 7 Mar 2019 00:51:28 -0800 Subject: [PATCH 3/3] play-json 2.7.1 (#308) # Pull Request Checklist * [x] Have you read through the [contributor guidelines](https://www.playframework.com/contributing)? * [x] Have you signed the [Typesafe CLA](https://www.typesafe.com/contribute/cla)? * [x] Have you [squashed your commits](https://www.playframework.com/documentation/latest/WorkingWithGit#Squashing-commits)? * [ ] Have you added copyright headers to new files? * [ ] Have you checked that both Scala and Java APIs are updated? * [ ] Have you updated the documentation for both Scala and Java sections? * [ ] Have you added tests for any changed functionality? ## Fixes Fixes #xxxx ## Purpose What does this PR do? ## Background Context Why did you take this approach? ## References Are there any relevant issues / PRs / mailing lists discussions? --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 682b8fe8..ddfcac43 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -23,7 +23,7 @@ object Dependencies { val scalaJava8Compat = Seq("org.scala-lang.modules" %% "scala-java8-compat" % "0.8.0") - val playJsonVersion = "2.7.0" + val playJsonVersion = "2.7.1" val playJson = Seq("com.typesafe.play" %% "play-json" % playJsonVersion) val slf4jApi = Seq("org.slf4j" % "slf4j-api" % "1.7.25")