From 56c5d92236daf8a8094429072ec70cf830fd10ac Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Mon, 21 Mar 2016 13:21:19 -0700 Subject: Add JMH to the benchmark framework. Add an example benchmark for OpenHashMap. --- test/benchmarks/.gitignore | 6 ++ test/benchmarks/README.md | 65 ++++++++++++++++++ test/benchmarks/build.sbt | 8 +++ test/benchmarks/project/plugins.sbt | 1 + .../collection/mutable/OpenHashMapBenchmark.scala | 76 ++++++++++++++++++++++ 5 files changed, 156 insertions(+) create mode 100644 test/benchmarks/.gitignore create mode 100644 test/benchmarks/README.md create mode 100644 test/benchmarks/build.sbt create mode 100644 test/benchmarks/project/plugins.sbt create mode 100644 test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala diff --git a/test/benchmarks/.gitignore b/test/benchmarks/.gitignore new file mode 100644 index 0000000000..6e3ddad6d2 --- /dev/null +++ b/test/benchmarks/.gitignore @@ -0,0 +1,6 @@ +/project/project/ +/project/target/ +/target/ + +# what appears to be a Scala IDE-generated file +.cache-main diff --git a/test/benchmarks/README.md b/test/benchmarks/README.md new file mode 100644 index 0000000000..99b358dd99 --- /dev/null +++ b/test/benchmarks/README.md @@ -0,0 +1,65 @@ +# Scala library benchmarks + +This directory is a standalone SBT project +that makes use of the [SBT plugin for JMH](https://github.com/ktoso/sbt-jmh), +with the usual directory structure: +source code for the benchmarks, which utilize [JMH](http://openjdk.java.net/projects/code-tools/jmh/), +should be placed in `src/main/scala`. + +The benchmarks require first building Scala into `../../build/pack`. +They can then be (built and) run from `sbt` with "`jmh:run`". +"`jmh:run -h`" displays the usual JMH options available. + +## some useful HotSpot options +Adding these to the `jmh:run` command line may help if you're using the HotSpot (Oracle, OpenJDK) compiler. +They require prefixing with `-jvmArgs`. +See [the Java documentation](http://docs.oracle.com/javase/8/docs/technotes/tools/unix/java.html) for more options. + +### viewing JIT compilation events +Adding `-XX:+PrintCompilation` shows when Java methods are being compiled or deoptimized. +At the most basic level, +these messages will tell you whether the code that you're measuring is still being tuned, +so that you know whether you're running enough warm-up iterations. +See [Kris Mok's notes](https://gist.github.com/rednaxelafx/1165804#file-notes-md) to interpret the output in detail. + +### consider GC events +If you're not explicitly performing `System.gc()` calls outside of your benchmarking code, +you should add the JVM option `-verbose:gc` to understand the effect that GCs may be having on your tests. + +### "diagnostic" options +These require the `-XX:+UnlockDiagnosticVMOptions` JVM option. + +#### viewing inlining events +Add `-XX:+PrintInlining`. + +#### viewing the disassembled code +To show the assembly code corresponding to the code generated by the JIT compiler for specific methods, +add `-XX:CompileCommand=print,scala.collection.mutable.OpenHashMap::*`, +for example, to show all of the methods in the `scala.collection.mutable.OpenHashMap` class. +If you're running OpenJDK, you may need to install the disassembler library (`hsdis-amd64.so` for the `amd64` architecture). +In Debian, this is available in the `libhsdis0-fcml` package. + +To show it for _all_ methods, add `-XX:+PrintAssembly`. +(This is usually excessive.) + +## useful reading +* [OpenJDK advice on microbenchmarks](https://wiki.openjdk.java.net/display/HotSpot/MicroBenchmarks) +* "[Measuring performance](http://docs.scala-lang.org/overviews/parallel-collections/performance.html)" of Scala parallel collections +* Brian Goetz's "Java theory and practice" articles: + * "[Dynamic compilation and performance measurement](http://www.ibm.com/developerworks/java/library/j-jtp12214/)" + * "[Anatomy of a flawed benchmark](http://www.ibm.com/developerworks/java/library/j-jtp02225/)" + +## legacy frameworks + +An older version of the benchmarking framework is still present in this directory, in the following locations: + +
+
bench
+
A script to run the old benchmarks.
+
source.list
+
A temporary file used by bench.
+
src/scala/
+
The older benchmarks, including the previous framework.
+
+ +Another, older set of benchmarks is present in `../benchmarking/`. diff --git a/test/benchmarks/build.sbt b/test/benchmarks/build.sbt new file mode 100644 index 0000000000..92a5fce177 --- /dev/null +++ b/test/benchmarks/build.sbt @@ -0,0 +1,8 @@ +scalaHome := Some(file("../../build/pack")) + +lazy val root = (project in file(".")). + enablePlugins(JmhPlugin). + settings( + name := "test-benchmarks", + version := "0.0.1" + ) diff --git a/test/benchmarks/project/plugins.sbt b/test/benchmarks/project/plugins.sbt new file mode 100644 index 0000000000..f5319fb187 --- /dev/null +++ b/test/benchmarks/project/plugins.sbt @@ -0,0 +1 @@ +addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.2.6") diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala new file mode 100644 index 0000000000..eeea8f6508 --- /dev/null +++ b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala @@ -0,0 +1,76 @@ +package scala.collection.mutable; + +import java.util.concurrent.TimeUnit +import org.openjdk.jmh.annotations._ + +private object OpenHashMapBenchmark { + /** State container for the `put()` bulk calling tests. + * + * Provides a thread-scoped map, so that allocation for the hash table will be done + * in the first warm-up iteration, not during measurement. + * + * Performs a GC after every invocation, so that only the GCs caused by the invocation + * contribute to the measurement. + */ + @State(Scope.Thread) + class BulkPutState { + val map = new OpenHashMap[Int,Int].empty + + @TearDown(Level.Invocation) + def teardown { map.clear(); System.gc() } + } +} + +/** Benchmark for the library's [[OpenHashMap]]. + * + * The `put()` calls are tested by looping to the size desired for the map; + * instead of using the JMH harness, which iterates for a fixed length of time. + */ +@BenchmarkMode(Array(Mode.AverageTime)) +@Threads(1) +@Fork(1) +@Warmup(iterations = 20) +@Measurement(iterations = 20) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +@State(Scope.Benchmark) +class OpenHashMapBenchmark { + import OpenHashMapBenchmark._ + + @Param(Array("100", "250", "1000", "2500", "10000", "25000", "100000", "250000", "1000000", "2500000", + "5000000", "7500000", "10000000", "25000000")) + var size: Int = _ + + /** Put elements into the given map. */ + private[this] def put_Int(map: OpenHashMap[Int,Int], from: Int, to: Int) { + var i = from + while (i <= to) { // using a `for` expression instead adds significant overhead + map.put(i, i) + i += 1 + } + } + + /** Test putting elements to a map of `Int` to `Int`. */ + @Benchmark + def put_Int(state: BulkPutState) { put_Int(state.map, 1, size) } + + /** Test putting and removing elements to a growing map of `Int` to `Int`. */ + @Benchmark + def put_remove_Int(state: BulkPutState) { + val blocks = 50 // should be a factor of `size` + val totalPuts = 2 * size // add twice as many, because we remove half of them + val blockSize: Int = totalPuts / blocks + var base = 0 + while (base < totalPuts) { + put_Int(state.map, base + 1, base + blockSize) + + // remove every other entry + var i = base + 1 + while (i <= base + blockSize) { + state.map.remove(i) + i += 2 + } + + base += blockSize + } + } +} -- cgit v1.2.3 From 303130b81599528db35d9612fff42cf7e570e15a Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Tue, 22 Mar 2016 23:12:31 -0700 Subject: Add a reference to Doug Lea's benchmarks. --- test/benchmarks/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/benchmarks/README.md b/test/benchmarks/README.md index 99b358dd99..aea72e90ed 100644 --- a/test/benchmarks/README.md +++ b/test/benchmarks/README.md @@ -44,10 +44,11 @@ To show it for _all_ methods, add `-XX:+PrintAssembly`. ## useful reading * [OpenJDK advice on microbenchmarks](https://wiki.openjdk.java.net/display/HotSpot/MicroBenchmarks) -* "[Measuring performance](http://docs.scala-lang.org/overviews/parallel-collections/performance.html)" of Scala parallel collections * Brian Goetz's "Java theory and practice" articles: * "[Dynamic compilation and performance measurement](http://www.ibm.com/developerworks/java/library/j-jtp12214/)" * "[Anatomy of a flawed benchmark](http://www.ibm.com/developerworks/java/library/j-jtp02225/)" +* [Doug Lea's JSR 166 benchmarks](http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/loops/) +* "[Measuring performance](http://docs.scala-lang.org/overviews/parallel-collections/performance.html)" of Scala parallel collections ## legacy frameworks -- cgit v1.2.3 From e1b58ccafc598c06b8011e3e0f411f6e91b99353 Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Wed, 23 Mar 2016 12:07:00 -0700 Subject: Add get() tests to OpenHashMap, reduce timing artifacts. In order to get a better exploration of the variance of tests in a limited time, I've reduced the number of measurement iterations and increased the number of forks. By sight, the measurement iterations seemed pretty consistent within a trial, whereas they would vary widely on occasional forks. I extended testing down to 50-entry maps, to explore the rise in service times that I was seeing at small scale. This is probably a timing artifact, from too-short invocations, since I'm using @Level.Invocation in the put() tests. To fix that, I enlarged the unit of testing, by creating multiple, sometimes thousands, of maps for the invocation to fill. This has also changed the test from filling a previously-filled map, to filling a new, but sufficiently sized map. The put()/remove() test now performs much worse (on a more realistic scenario). This also adds a couple tests for calling get() against a map that's been filled only with put()s, or with a mix of put() and remove(). --- .../collection/mutable/OpenHashMapBenchmark.scala | 179 +++++++++++++++++---- 1 file changed, 144 insertions(+), 35 deletions(-) diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala index eeea8f6508..73ab5e40d0 100644 --- a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala +++ b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala @@ -2,46 +2,104 @@ package scala.collection.mutable; import java.util.concurrent.TimeUnit import org.openjdk.jmh.annotations._ +import org.openjdk.jmh.infra.Blackhole +import org.openjdk.jmh.infra.BenchmarkParams +/** Utilities for the [[OpenHashMapBenchmark]]. + * + * The method calls are tested by looping to the size desired for the map; + * instead of using the JMH harness, which iterates for a fixed length of time. + */ private object OpenHashMapBenchmark { /** State container for the `put()` bulk calling tests. * - * Provides a thread-scoped map, so that allocation for the hash table will be done - * in the first warm-up iteration, not during measurement. + * Provides an array of adequately-sized, empty maps to each invocation, + * so that hash table allocation won't be done during measurement. * - * Performs a GC after every invocation, so that only the GCs caused by the invocation - * contribute to the measurement. + * Empties the map and performs a GC after every invocation, + * so that only the GCs caused by the invocation contribute to the measurement. */ @State(Scope.Thread) + @AuxCounters class BulkPutState { + /** A lower-bound estimate of the number of nanoseconds per `put()` call */ + private[this] val nanosPerPut: Double = 5 + + /** Minimum number of nanoseconds per invocation, so as to avoid timing artifacts. */ + private[this] val minNanosPerInvocation = 1000000 // one millisecond + + /** The minimum number of `put()` calls to make per invocation, so as to avoid timing artifacts. */ + private[this] val minPutsPerInvocation = minNanosPerInvocation / nanosPerPut + + /** Size of the maps created in this trial. */ + private[this] var size: Int = _ + + /** Number of maps created in each invocation; the size of `maps`. */ + private[this] var n: Int = _ + + /** Number of operations performed in the current invocation. */ + var operations: Int = _ + + var maps: Array[OpenHashMap[Int,Int]] = null + + @Setup + def threadSetup(params: BenchmarkParams) { + size = params.getParam("size").toInt + n = math.ceil(minPutsPerInvocation / size).toInt + maps = new Array(n) + } + + @Setup(Level.Iteration) + def iterationSetup { + operations = 0 + } + + @Setup(Level.Invocation) + def setup { + for (i <- 0 until n) maps(i) = new OpenHashMap[Int,Int](size) + operations += size * n + System.gc() // clean up after last invocation + } + } + + /** State container for the `get()` bulk calling tests. + * + * Provides a thread-scoped map of the expected size. + * Performs a GC after loading the map. + */ + @State(Scope.Thread) + class BulkGetState { val map = new OpenHashMap[Int,Int].empty - - @TearDown(Level.Invocation) - def teardown { map.clear(); System.gc() } + + /** Load the map with keys from `1` to `size`. */ + @Setup + def setup(params: BenchmarkParams) { + val size = params.getParam("size").toInt + put_Int(map, 1, size) + System.gc() + } } -} -/** Benchmark for the library's [[OpenHashMap]]. - * - * The `put()` calls are tested by looping to the size desired for the map; - * instead of using the JMH harness, which iterates for a fixed length of time. - */ -@BenchmarkMode(Array(Mode.AverageTime)) -@Threads(1) -@Fork(1) -@Warmup(iterations = 20) -@Measurement(iterations = 20) -@OutputTimeUnit(TimeUnit.MICROSECONDS) -@State(Scope.Benchmark) -class OpenHashMapBenchmark { - import OpenHashMapBenchmark._ + /** State container for the `get()` bulk calling tests with deleted entries. + * + * Provides a thread-scoped map of the expected size, from which entries have been removed. + * Performs a GC after loading the map. + */ + @State(Scope.Thread) + class BulkRemovedGetState { + val map = new OpenHashMap[Int,Int].empty - @Param(Array("100", "250", "1000", "2500", "10000", "25000", "100000", "250000", "1000000", "2500000", - "5000000", "7500000", "10000000", "25000000")) - var size: Int = _ + /** Load the map with keys from `1` to `size`, removing half of them. */ + @Setup + def setup(params: BenchmarkParams) { + val size = params.getParam("size").toInt + put_remove_Int(map, size) + System.gc() + } + } /** Put elements into the given map. */ - private[this] def put_Int(map: OpenHashMap[Int,Int], from: Int, to: Int) { + private def put_Int(map: OpenHashMap[Int,Int], from: Int, to: Int) { var i = from while (i <= to) { // using a `for` expression instead adds significant overhead map.put(i, i) @@ -49,28 +107,79 @@ class OpenHashMapBenchmark { } } - /** Test putting elements to a map of `Int` to `Int`. */ - @Benchmark - def put_Int(state: BulkPutState) { put_Int(state.map, 1, size) } - - /** Test putting and removing elements to a growing map of `Int` to `Int`. */ - @Benchmark - def put_remove_Int(state: BulkPutState) { + /** Put elements into the given map, removing half of them as they're added. + * + * @param size number of entries to leave in the map on return + */ + def put_remove_Int(map: OpenHashMap[Int,Int], size: Int) { val blocks = 50 // should be a factor of `size` val totalPuts = 2 * size // add twice as many, because we remove half of them val blockSize: Int = totalPuts / blocks var base = 0 while (base < totalPuts) { - put_Int(state.map, base + 1, base + blockSize) + put_Int(map, base + 1, base + blockSize) // remove every other entry var i = base + 1 while (i <= base + blockSize) { - state.map.remove(i) + map.remove(i) i += 2 } base += blockSize } } + + /** Get elements from the given map. */ + def get_Int(map: OpenHashMap[Int,Int], size: Int, bh: Blackhole) { + var i = 1 + while (i <= size) { + bh.consume(map.get(i).getOrElse(0)) + i += 1 + } + } +} + +/** Benchmark for the library's [[OpenHashMap]]. */ +@BenchmarkMode(Array(Mode.AverageTime)) +@Fork(10) +@Threads(1) +@Warmup(iterations = 20) +@Measurement(iterations = 6) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Benchmark) +class OpenHashMapBenchmark { + import OpenHashMapBenchmark._ + + @Param(Array("50", "100", "250", "1000", "2500", "10000", "25000", "100000", "250000", "1000000", "2500000", + "5000000", "7500000", "10000000", "25000000")) + var size: Int = _ + + /** Test putting elements to a map of `Int` to `Int`. */ + @Benchmark + def put_Int(state: BulkPutState) { + var i = 0 + while (i < state.maps.length) { + OpenHashMapBenchmark.put_Int(state.maps(i), 1, size) + i += 1 + } + } + + /** Test putting and removing elements to a growing map of `Int` to `Int`. */ + @Benchmark + def put_remove_Int(state: BulkPutState) { + var i = 0 + while (i < state.maps.length) { + OpenHashMapBenchmark.put_remove_Int(state.maps(i), size) + i += 1 + } + } + + /** Test getting elements from a map of `Int` to `Int`. */ + @Benchmark + def put_get_Int(state: BulkGetState, bh: Blackhole) = OpenHashMapBenchmark.get_Int(state.map, size, bh) + + /** Test getting elements from a map of `Int` to `Int` from which elements have been removed. */ + @Benchmark + def put_remove_get_Int(state: BulkRemovedGetState, bh: Blackhole) = OpenHashMapBenchmark.get_Int(state.map, size, bh) } -- cgit v1.2.3 From b88933eb84f1f1f5215b0feb43f4ecfc12c8847d Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Fri, 25 Mar 2016 21:44:50 -0700 Subject: Benchmark the OpenHashMap memory usage. Also add sbteclipse to the benchmark project. --- test/benchmarks/.gitignore | 8 ++++++ test/benchmarks/build.sbt | 5 +++- test/benchmarks/project/plugins.sbt | 1 + .../collection/mutable/OpenHashMapBenchmark.scala | 32 +++++++++++++++------- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/test/benchmarks/.gitignore b/test/benchmarks/.gitignore index 6e3ddad6d2..ce4d893417 100644 --- a/test/benchmarks/.gitignore +++ b/test/benchmarks/.gitignore @@ -4,3 +4,11 @@ # what appears to be a Scala IDE-generated file .cache-main + +# standard Eclipse output directory +/bin/ + +# sbteclipse-generated Eclipse files +/.classpath +/.project +/.settings/ diff --git a/test/benchmarks/build.sbt b/test/benchmarks/build.sbt index 92a5fce177..2959e4986a 100644 --- a/test/benchmarks/build.sbt +++ b/test/benchmarks/build.sbt @@ -1,8 +1,11 @@ scalaHome := Some(file("../../build/pack")) +scalaVersion := "2.11.8" +scalacOptions += "-feature" lazy val root = (project in file(".")). enablePlugins(JmhPlugin). settings( name := "test-benchmarks", - version := "0.0.1" + version := "0.0.1", + libraryDependencies += "org.openjdk.jol" % "jol-core" % "0.4" ) diff --git a/test/benchmarks/project/plugins.sbt b/test/benchmarks/project/plugins.sbt index f5319fb187..e11aa29f3b 100644 --- a/test/benchmarks/project/plugins.sbt +++ b/test/benchmarks/project/plugins.sbt @@ -1 +1,2 @@ +addSbtPlugin("com.typesafe.sbteclipse" % "sbteclipse-plugin" % "4.0.0") addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.2.6") diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala index 73ab5e40d0..13be9e6206 100644 --- a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala +++ b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala @@ -4,6 +4,9 @@ import java.util.concurrent.TimeUnit import org.openjdk.jmh.annotations._ import org.openjdk.jmh.infra.Blackhole import org.openjdk.jmh.infra.BenchmarkParams +import org.openjdk.jol.info.GraphLayout +import org.openjdk.jol.info.GraphWalker +import org.openjdk.jol.info.GraphVisitor /** Utilities for the [[OpenHashMapBenchmark]]. * @@ -15,9 +18,11 @@ private object OpenHashMapBenchmark { * * Provides an array of adequately-sized, empty maps to each invocation, * so that hash table allocation won't be done during measurement. - * - * Empties the map and performs a GC after every invocation, + * Provides enough maps to make each invocation long enough to avoid timing artifacts. + * Performs a GC after re-creating the empty maps before every invocation, * so that only the GCs caused by the invocation contribute to the measurement. + * + * Records the memory used by all the maps in the last invocation of each iteration. */ @State(Scope.Thread) @AuxCounters @@ -28,24 +33,25 @@ private object OpenHashMapBenchmark { /** Minimum number of nanoseconds per invocation, so as to avoid timing artifacts. */ private[this] val minNanosPerInvocation = 1000000 // one millisecond - /** The minimum number of `put()` calls to make per invocation, so as to avoid timing artifacts. */ - private[this] val minPutsPerInvocation = minNanosPerInvocation / nanosPerPut - /** Size of the maps created in this trial. */ private[this] var size: Int = _ - /** Number of maps created in each invocation; the size of `maps`. */ - private[this] var n: Int = _ + /** Total number of entries in all of the `maps` combined. */ + var mapEntries: Int = _ /** Number of operations performed in the current invocation. */ var operations: Int = _ + /** Bytes of memory used in the object graphs of all the maps. */ + var memory: Long = _ + var maps: Array[OpenHashMap[Int,Int]] = null @Setup def threadSetup(params: BenchmarkParams) { size = params.getParam("size").toInt - n = math.ceil(minPutsPerInvocation / size).toInt + val n = math.ceil(minNanosPerInvocation / (nanosPerPut * size)).toInt + mapEntries = size * n maps = new Array(n) } @@ -56,10 +62,16 @@ private object OpenHashMapBenchmark { @Setup(Level.Invocation) def setup { - for (i <- 0 until n) maps(i) = new OpenHashMap[Int,Int](size) - operations += size * n + for (i <- 0 until maps.length) maps(i) = new OpenHashMap[Int,Int](size) + operations += mapEntries System.gc() // clean up after last invocation } + + @TearDown(Level.Iteration) + def iterationTeardown { + // limit to smaller cases to avoid OOM + memory = if (mapEntries <= 1000000) GraphLayout.parseInstance(maps(0), maps.tail).totalSize else 0 + } } /** State container for the `get()` bulk calling tests. -- cgit v1.2.3 From 2e8fb12f6d92e6021131461285b9c28909584d04 Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Sun, 27 Mar 2016 11:59:13 -0700 Subject: Add a JMH runner class to the library benchmark framework. --- test/benchmarks/README.md | 49 +++++++-- .../src/main/scala/benchmark/JmhRunner.scala | 16 +++ .../collection/mutable/OpenHashMapRunner.scala | 111 +++++++++++++++++++++ 3 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 test/benchmarks/src/main/scala/benchmark/JmhRunner.scala create mode 100644 test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala diff --git a/test/benchmarks/README.md b/test/benchmarks/README.md index aea72e90ed..07e72f09a1 100644 --- a/test/benchmarks/README.md +++ b/test/benchmarks/README.md @@ -1,17 +1,48 @@ # Scala library benchmarks -This directory is a standalone SBT project -that makes use of the [SBT plugin for JMH](https://github.com/ktoso/sbt-jmh), -with the usual directory structure: -source code for the benchmarks, which utilize [JMH](http://openjdk.java.net/projects/code-tools/jmh/), -should be placed in `src/main/scala`. +This directory is a standalone SBT project, within the Scala project, +that makes use of the [SBT plugin](https://github.com/ktoso/sbt-jmh) for [JMH](http://openjdk.java.net/projects/code-tools/jmh/). -The benchmarks require first building Scala into `../../build/pack`. -They can then be (built and) run from `sbt` with "`jmh:run`". -"`jmh:run -h`" displays the usual JMH options available. +## running a benchmark + +The benchmarks require first building Scala into `../../build/pack`, using Ant. + +You'll then need to know the fully-qualified name of the benchmark runner class. +The benchmarking classes are organized under `src/main/scala`, +in the same package hierarchy as the classes that they test. +Assuming that we're benchmarking `scala.collection.mutable.OpenHashMap`, +the benchmark runner would likely be named `scala.collection.mutable.OpenHashMapRunner`. +Using this example, one would simply run + + jmh:runMain scala.collection.mutable.OpenHashMapRunner + +in SBT. +SBT should be run _from this directory_. + +The JMH results can be found under `target/jmh-results/`. +`target` gets deleted on an SBT `clean`, +so you should copy these files out of `target` if you wish to preserve them. + +## creating a benchmark and runner + +The benchmarking classes use the same package hierarchy as the classes that they test +in order to make it easy to expose, in package scope, members of the class under test, +should that be necessary for benchmarking. + +There are two types of classes in the source directory: +those suffixed "`Benchmark`" and those suffixed "`Runner`". +The former are benchmarks that can be run directly using `jmh:run`; +however, they are normally run from a corresponding class of the latter type, +which is run using `jmh:runMain` (as described above). +This …`Runner` class is useful for setting appropriate JMH command options, +and for processing the JMH results into files that can be read by other tools, such as Gnuplot. + +The `benchmark.JmhRunner` trait should be woven into any runner class, for the standard behavior that it provides. +This includes creating output files in a subdirectory of `target/jmh-results` +derived from the fully-qualified package name of the `Runner` class. ## some useful HotSpot options -Adding these to the `jmh:run` command line may help if you're using the HotSpot (Oracle, OpenJDK) compiler. +Adding these to the `jmh:run` or `jmh:runMain` command line may help if you're using the HotSpot (Oracle, OpenJDK) compiler. They require prefixing with `-jvmArgs`. See [the Java documentation](http://docs.oracle.com/javase/8/docs/technotes/tools/unix/java.html) for more options. diff --git a/test/benchmarks/src/main/scala/benchmark/JmhRunner.scala b/test/benchmarks/src/main/scala/benchmark/JmhRunner.scala new file mode 100644 index 0000000000..cc75be529d --- /dev/null +++ b/test/benchmarks/src/main/scala/benchmark/JmhRunner.scala @@ -0,0 +1,16 @@ +package benchmark + +import java.io.File + +/** Common code for JMH runner objects. */ +trait JmhRunner { + private[this] val parentDirectory = new File("target", "jmh-results") + + /** Return the output directory for this class, creating the directory if necessary. */ + protected def outputDirectory: File = { + val subdir = getClass.getPackage.getName.replace('.', File.separatorChar) + val dir = new File(parentDirectory, subdir) + if (!dir.isDirectory) dir.mkdirs() + dir + } +} diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala new file mode 100644 index 0000000000..c139c55933 --- /dev/null +++ b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala @@ -0,0 +1,111 @@ +package scala.collection.mutable + +import java.io.BufferedWriter +import java.io.File +import java.io.FileOutputStream +import java.io.OutputStreamWriter +import java.io.PrintWriter +import scala.collection.JavaConversions +import scala.language.existentials +import org.openjdk.jmh.results.RunResult +import org.openjdk.jmh.runner.Runner +import org.openjdk.jmh.runner.options.CommandLineOptions +import org.openjdk.jmh.runner.options.Options +import benchmark.JmhRunner +import org.openjdk.jmh.runner.options.OptionsBuilder +import org.openjdk.jmh.runner.options.VerboseMode + +/** Replacement JMH application that runs the [[OpenHashMap]] benchmark. + * + * Outputs the results in a form consumable by a Gnuplot script. + */ +object OpenHashMapRunner extends JmhRunner { + /** File that will be created for the output data set. */ + private[this] val outputFile = new File(outputDirectory, "OpenHashMap.dat") + + /** Qualifier to add to the name of a memory usage data set. */ + private[this] val memoryDatasetQualifer = " memory" + + /** Name of the JMH parameter for the number of map entries per invocation. */ + private[this] val sizeParamName = "size" + + /** Name of the JMH auxiliary counter that collects operation counts. */ + private[this] val operationsAuxCounterName = "operations" + + /** Name of the JMH auxiliary counter that collects memory usage. */ + private[this] val memoryAuxCounterName = "memory" + + /** Name of the JMH auxiliary counter that collects the number of map entries. */ + private[this] val entriesAuxCounterName = "mapEntries" + + def main(args: Array[String]) { + import scala.collection.JavaConversions._ + import scala.language.existentials + + val opts = new CommandLineOptions(args: _*) + var builder = new OptionsBuilder().parent(opts) + .jvmArgsPrepend("-Xmx6000m") + if (!opts.verbosity.hasValue) builder = builder.verbosity(VerboseMode.SILENT) + + val results = new Runner(builder.build).run() + + // Sort the results + + /** Map from data set name to data set. */ + val datasetByName = Map.empty[String, Set[RunResult]] + + /** Ordering for the results within a data set. Orders by increasing number of map entries. */ + val ordering = Ordering.by[RunResult, Int](_.getParams.getParam(sizeParamName).toInt) + + def addToDataset(result: RunResult, key: String): Unit = + datasetByName.get(key) + .getOrElse({ val d = SortedSet.empty(ordering); datasetByName.put(key, d); d }) += result + + results.foreach { result: RunResult ⇒ + addToDataset(result, result.getPrimaryResult.getLabel) + + // Create another data set for trials that track memory usage + if (result.getSecondaryResults.containsKey(memoryAuxCounterName)) + addToDataset(result, result.getPrimaryResult.getLabel + memoryDatasetQualifer) + } + + //TODO Write out test parameters + // val jvm = params.getJvm + // val jvmArgs = params.getJvmArgs.mkString(" ") + + val f = new PrintWriter(outputFile, "UTF-8") + try { + datasetByName.foreach(_ match { case (label: String, dataset: Iterable[RunResult]) ⇒ { + f.format("# [%s]\n", label) + + val isMemoryUsageDataset = label.contains(memoryDatasetQualifer) + dataset.foreach { result ⇒ + val size = result.getParams.getParam(sizeParamName) + val secondaryResults = result.getSecondaryResults + if (isMemoryUsageDataset) { + val memoryResult = secondaryResults.get(memoryAuxCounterName) + val entriesResult = secondaryResults.get(entriesAuxCounterName) + f.format("%s %f %f %f %f\n", size, + Double.box(entriesResult.getScore), Double.box(entriesResult.getStatistics.getStandardDeviation), + Double.box(memoryResult.getScore), Double.box(memoryResult.getStatistics.getStandardDeviation)) + } + else { + if (secondaryResults.containsKey(operationsAuxCounterName)) { + val operationsResult = secondaryResults.get(operationsAuxCounterName) + f.format("%s %f %f\n", size, + Double.box(operationsResult.getScore), Double.box(operationsResult.getStatistics.getStandardDeviation)) + } else { + val primary = result.getPrimaryResult + f.format("%s %f %f\n", size, + Double.box(primary.getScore), Double.box(primary.getStatistics.getStandardDeviation)) + } + } + } + + f.println(); f.println() // data set separator + }}) + } finally { + f.close() + } + } +} -- cgit v1.2.3 From cd7be12a35fd2cf7d0448d59b6f43e4165f43db4 Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Mon, 28 Mar 2016 13:54:43 -0700 Subject: Improve the OpenHashMapBenchmark run times. For the warm-up invocations, suppress setup and teardown that is only needed for the measurement iterations. Reduce the number of forks. --- .../collection/mutable/OpenHashMapBenchmark.scala | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala index 13be9e6206..78e160a713 100644 --- a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala +++ b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala @@ -7,6 +7,8 @@ import org.openjdk.jmh.infra.BenchmarkParams import org.openjdk.jol.info.GraphLayout import org.openjdk.jol.info.GraphWalker import org.openjdk.jol.info.GraphVisitor +import org.openjdk.jmh.infra.IterationParams +import org.openjdk.jmh.runner.IterationType /** Utilities for the [[OpenHashMapBenchmark]]. * @@ -61,16 +63,21 @@ private object OpenHashMapBenchmark { } @Setup(Level.Invocation) - def setup { + def setup(params: IterationParams) { for (i <- 0 until maps.length) maps(i) = new OpenHashMap[Int,Int](size) - operations += mapEntries - System.gc() // clean up after last invocation + + if (params.getType == IterationType.MEASUREMENT) { + operations += mapEntries + System.gc() // clean up after last invocation + } } @TearDown(Level.Iteration) - def iterationTeardown { - // limit to smaller cases to avoid OOM - memory = if (mapEntries <= 1000000) GraphLayout.parseInstance(maps(0), maps.tail).totalSize else 0 + def iterationTeardown(params: IterationParams) { + if (params.getType == IterationType.MEASUREMENT) { + // limit to smaller cases to avoid OOM + memory = if (mapEntries <= 1000000) GraphLayout.parseInstance(maps(0), maps.tail).totalSize else 0 + } } } @@ -154,7 +161,7 @@ private object OpenHashMapBenchmark { /** Benchmark for the library's [[OpenHashMap]]. */ @BenchmarkMode(Array(Mode.AverageTime)) -@Fork(10) +@Fork(6) @Threads(1) @Warmup(iterations = 20) @Measurement(iterations = 6) -- cgit v1.2.3 From 00cbba19710b23f856f6c4a29e40a82a4ee364a9 Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Mon, 2 May 2016 22:47:58 -0700 Subject: Address JMH benchmark reviewer's issues. Besides tweaks to the documentation, this tests smaller (25-element) maps, and rewrites OpenHashMapRunner in more idiomatic Scala. --- test/benchmarks/README.md | 36 ++++++---- .../collection/mutable/OpenHashMapBenchmark.scala | 2 +- .../collection/mutable/OpenHashMapRunner.scala | 82 ++++++++++------------ 3 files changed, 61 insertions(+), 59 deletions(-) diff --git a/test/benchmarks/README.md b/test/benchmarks/README.md index 07e72f09a1..370d610bc4 100644 --- a/test/benchmarks/README.md +++ b/test/benchmarks/README.md @@ -3,9 +3,11 @@ This directory is a standalone SBT project, within the Scala project, that makes use of the [SBT plugin](https://github.com/ktoso/sbt-jmh) for [JMH](http://openjdk.java.net/projects/code-tools/jmh/). -## running a benchmark +## Running a benchmark -The benchmarks require first building Scala into `../../build/pack`, using Ant. +The benchmarks require first building Scala into `../../build/pack` with `ant`. +If you want to build with `sbt dist/mkPack` instead, +you'll need to change `scalaHome` in this project. You'll then need to know the fully-qualified name of the benchmark runner class. The benchmarking classes are organized under `src/main/scala`, @@ -23,14 +25,14 @@ The JMH results can be found under `target/jmh-results/`. `target` gets deleted on an SBT `clean`, so you should copy these files out of `target` if you wish to preserve them. -## creating a benchmark and runner +## Creating a benchmark and runner The benchmarking classes use the same package hierarchy as the classes that they test in order to make it easy to expose, in package scope, members of the class under test, should that be necessary for benchmarking. There are two types of classes in the source directory: -those suffixed "`Benchmark`" and those suffixed "`Runner`". +those suffixed `Benchmark` and those suffixed `Runner`. The former are benchmarks that can be run directly using `jmh:run`; however, they are normally run from a corresponding class of the latter type, which is run using `jmh:runMain` (as described above). @@ -41,39 +43,45 @@ The `benchmark.JmhRunner` trait should be woven into any runner class, for the s This includes creating output files in a subdirectory of `target/jmh-results` derived from the fully-qualified package name of the `Runner` class. -## some useful HotSpot options +## Some useful HotSpot options Adding these to the `jmh:run` or `jmh:runMain` command line may help if you're using the HotSpot (Oracle, OpenJDK) compiler. They require prefixing with `-jvmArgs`. See [the Java documentation](http://docs.oracle.com/javase/8/docs/technotes/tools/unix/java.html) for more options. -### viewing JIT compilation events +### Viewing JIT compilation events Adding `-XX:+PrintCompilation` shows when Java methods are being compiled or deoptimized. At the most basic level, these messages will tell you whether the code that you're measuring is still being tuned, so that you know whether you're running enough warm-up iterations. See [Kris Mok's notes](https://gist.github.com/rednaxelafx/1165804#file-notes-md) to interpret the output in detail. -### consider GC events +### Consider GC events If you're not explicitly performing `System.gc()` calls outside of your benchmarking code, you should add the JVM option `-verbose:gc` to understand the effect that GCs may be having on your tests. -### "diagnostic" options +### "Diagnostic" options These require the `-XX:+UnlockDiagnosticVMOptions` JVM option. -#### viewing inlining events +#### Viewing inlining events Add `-XX:+PrintInlining`. -#### viewing the disassembled code +#### Viewing the disassembled code +If you're running OpenJDK or Oracle JVM, +you may need to install the disassembler library (`hsdis-amd64.so` for the `amd64` architecture). +In Debian, this is available in +the `libhsdis0-fcml` package. +For an Oracle (or other compatible) JVM not set up by your distribution, +you may also need to copy or link the disassembler library +to the `jre/lib/`_`architecture`_ directory inside your JVM installation directory. + To show the assembly code corresponding to the code generated by the JIT compiler for specific methods, add `-XX:CompileCommand=print,scala.collection.mutable.OpenHashMap::*`, for example, to show all of the methods in the `scala.collection.mutable.OpenHashMap` class. -If you're running OpenJDK, you may need to install the disassembler library (`hsdis-amd64.so` for the `amd64` architecture). -In Debian, this is available in the `libhsdis0-fcml` package. To show it for _all_ methods, add `-XX:+PrintAssembly`. (This is usually excessive.) -## useful reading +## Useful reading * [OpenJDK advice on microbenchmarks](https://wiki.openjdk.java.net/display/HotSpot/MicroBenchmarks) * Brian Goetz's "Java theory and practice" articles: * "[Dynamic compilation and performance measurement](http://www.ibm.com/developerworks/java/library/j-jtp12214/)" @@ -81,7 +89,7 @@ To show it for _all_ methods, add `-XX:+PrintAssembly`. * [Doug Lea's JSR 166 benchmarks](http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/loops/) * "[Measuring performance](http://docs.scala-lang.org/overviews/parallel-collections/performance.html)" of Scala parallel collections -## legacy frameworks +## Legacy frameworks An older version of the benchmarking framework is still present in this directory, in the following locations: diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala index 78e160a713..26e26b3065 100644 --- a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala +++ b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapBenchmark.scala @@ -170,7 +170,7 @@ private object OpenHashMapBenchmark { class OpenHashMapBenchmark { import OpenHashMapBenchmark._ - @Param(Array("50", "100", "250", "1000", "2500", "10000", "25000", "100000", "250000", "1000000", "2500000", + @Param(Array("25", "50", "100", "250", "1000", "2500", "10000", "25000", "100000", "250000", "1000000", "2500000", "5000000", "7500000", "10000000", "25000000")) var size: Int = _ diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala index c139c55933..1a58b18ee9 100644 --- a/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala +++ b/test/benchmarks/src/main/scala/scala/collection/mutable/OpenHashMapRunner.scala @@ -14,6 +14,7 @@ import org.openjdk.jmh.runner.options.Options import benchmark.JmhRunner import org.openjdk.jmh.runner.options.OptionsBuilder import org.openjdk.jmh.runner.options.VerboseMode +import org.openjdk.jmh.results.Result /** Replacement JMH application that runs the [[OpenHashMap]] benchmark. * @@ -24,27 +25,35 @@ object OpenHashMapRunner extends JmhRunner { private[this] val outputFile = new File(outputDirectory, "OpenHashMap.dat") /** Qualifier to add to the name of a memory usage data set. */ - private[this] val memoryDatasetQualifer = " memory" + private[this] val memoryDatasetQualifier = "-memory" - /** Name of the JMH parameter for the number of map entries per invocation. */ - private[this] val sizeParamName = "size" + private[this] implicit class MyRunResult(r: RunResult) { + /** Return the dataset label. */ + def label = r.getPrimaryResult.getLabel - /** Name of the JMH auxiliary counter that collects operation counts. */ - private[this] val operationsAuxCounterName = "operations" + /** Return the value of the JMH parameter for the number of map entries per invocation. */ + def size: String = r.getParams.getParam("size") - /** Name of the JMH auxiliary counter that collects memory usage. */ - private[this] val memoryAuxCounterName = "memory" + /** Return the operation counts. */ + def operations = Option(r.getSecondaryResults.get("operations")) + + /** Return the number of map entries. */ + def entries = r.getSecondaryResults.get("mapEntries") + + /** Return the memory usage. */ + def memory = Option(r.getSecondaryResults.get("memory")) + } + + /** Return the statistics of the given result as a string. */ + private[this] def stats(r: Result[_]) = r.getScore + " " + r.getStatistics.getStandardDeviation - /** Name of the JMH auxiliary counter that collects the number of map entries. */ - private[this] val entriesAuxCounterName = "mapEntries" def main(args: Array[String]) { import scala.collection.JavaConversions._ import scala.language.existentials val opts = new CommandLineOptions(args: _*) - var builder = new OptionsBuilder().parent(opts) - .jvmArgsPrepend("-Xmx6000m") + var builder = new OptionsBuilder().parent(opts).jvmArgsPrepend("-Xmx6000m") if (!opts.verbosity.hasValue) builder = builder.verbosity(VerboseMode.SILENT) val results = new Runner(builder.build).run() @@ -55,18 +64,17 @@ object OpenHashMapRunner extends JmhRunner { val datasetByName = Map.empty[String, Set[RunResult]] /** Ordering for the results within a data set. Orders by increasing number of map entries. */ - val ordering = Ordering.by[RunResult, Int](_.getParams.getParam(sizeParamName).toInt) + val ordering = Ordering.by[RunResult, Int](_.size.toInt) - def addToDataset(result: RunResult, key: String): Unit = - datasetByName.get(key) - .getOrElse({ val d = SortedSet.empty(ordering); datasetByName.put(key, d); d }) += result + def addToDataset(key: String, result: RunResult): Unit = + datasetByName.getOrElseUpdate(key, SortedSet.empty(ordering)) += result - results.foreach { result: RunResult ⇒ - addToDataset(result, result.getPrimaryResult.getLabel) + results.foreach { result => + addToDataset(result.label, result) // Create another data set for trials that track memory usage - if (result.getSecondaryResults.containsKey(memoryAuxCounterName)) - addToDataset(result, result.getPrimaryResult.getLabel + memoryDatasetQualifer) + if (result.memory.isDefined) + addToDataset(result.label + memoryDatasetQualifier, result) } //TODO Write out test parameters @@ -75,31 +83,17 @@ object OpenHashMapRunner extends JmhRunner { val f = new PrintWriter(outputFile, "UTF-8") try { - datasetByName.foreach(_ match { case (label: String, dataset: Iterable[RunResult]) ⇒ { - f.format("# [%s]\n", label) - - val isMemoryUsageDataset = label.contains(memoryDatasetQualifer) - dataset.foreach { result ⇒ - val size = result.getParams.getParam(sizeParamName) - val secondaryResults = result.getSecondaryResults - if (isMemoryUsageDataset) { - val memoryResult = secondaryResults.get(memoryAuxCounterName) - val entriesResult = secondaryResults.get(entriesAuxCounterName) - f.format("%s %f %f %f %f\n", size, - Double.box(entriesResult.getScore), Double.box(entriesResult.getStatistics.getStandardDeviation), - Double.box(memoryResult.getScore), Double.box(memoryResult.getStatistics.getStandardDeviation)) - } - else { - if (secondaryResults.containsKey(operationsAuxCounterName)) { - val operationsResult = secondaryResults.get(operationsAuxCounterName) - f.format("%s %f %f\n", size, - Double.box(operationsResult.getScore), Double.box(operationsResult.getStatistics.getStandardDeviation)) - } else { - val primary = result.getPrimaryResult - f.format("%s %f %f\n", size, - Double.box(primary.getScore), Double.box(primary.getStatistics.getStandardDeviation)) - } - } + datasetByName.foreach(_ match { case (label: String, dataset: Iterable[RunResult]) => { + f.println(s"# [$label]") + + val isMemoryUsageDataset = label.endsWith(memoryDatasetQualifier) + dataset.foreach { r => + f.println(r.size + " " + ( + if (isMemoryUsageDataset) + stats(r.entries) + " " + stats(r.memory.get) + else + stats(r.operations getOrElse r.getPrimaryResult) + )) } f.println(); f.println() // data set separator -- cgit v1.2.3 From 486821b845ccaa0d02dd402fb3532d6d82055015 Mon Sep 17 00:00:00 2001 From: Performant Data LLC Date: Tue, 3 May 2016 10:52:28 -0700 Subject: Enable full compiler optimizations in JMH benchmarking. --- test/benchmarks/build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/benchmarks/build.sbt b/test/benchmarks/build.sbt index 2959e4986a..4806ecdde8 100644 --- a/test/benchmarks/build.sbt +++ b/test/benchmarks/build.sbt @@ -1,6 +1,6 @@ scalaHome := Some(file("../../build/pack")) scalaVersion := "2.11.8" -scalacOptions += "-feature" +scalacOptions ++= Seq("-feature", "-Yopt:l:classpath") lazy val root = (project in file(".")). enablePlugins(JmhPlugin). -- cgit v1.2.3 From 73ca44be579e5100706d174f18025fc4487e9cb9 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 16 May 2016 16:24:50 -0700 Subject: SI-4625 Recognize App in script Cheap name test: if the script object extends "App", take it for a main-bearing parent. Note that if `-Xscript` is not `Main`, the default, then the source is taken as a snippet and there is no attempt to locate an existing `main` method. --- src/compiler/scala/tools/nsc/ast/parser/Parsers.scala | 6 +++++- test/files/run/t4625.check | 1 + test/files/run/t4625.scala | 7 +++++++ test/files/run/t4625.script | 5 +++++ 4 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t4625.check create mode 100644 test/files/run/t4625.scala create mode 100644 test/files/run/t4625.script diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index c04d305f9e..7af5c505de 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -380,11 +380,15 @@ self => case DefDef(_, nme.main, Nil, List(_), _, _) => true case _ => false } + def isApp(t: Tree) = t match { + case Template(ps, _, _) => ps.exists { case Ident(x) if x.decoded == "App" => true ; case _ => false } + case _ => false + } /* For now we require there only be one top level object. */ var seenModule = false val newStmts = stmts collect { case t @ Import(_, _) => t - case md @ ModuleDef(mods, name, template) if !seenModule && (md exists isMainMethod) => + case md @ ModuleDef(mods, name, template) if !seenModule && (isApp(template) || md.exists(isMainMethod)) => seenModule = true /* This slightly hacky situation arises because we have no way to communicate * back to the scriptrunner what the name of the program is. Even if we were diff --git a/test/files/run/t4625.check b/test/files/run/t4625.check new file mode 100644 index 0000000000..e4a4d15b87 --- /dev/null +++ b/test/files/run/t4625.check @@ -0,0 +1 @@ +Test ran. diff --git a/test/files/run/t4625.scala b/test/files/run/t4625.scala new file mode 100644 index 0000000000..44f6225220 --- /dev/null +++ b/test/files/run/t4625.scala @@ -0,0 +1,7 @@ + +import scala.tools.partest.ScriptTest + +object Test extends ScriptTest { + // must be called Main to get probing treatment in parser + override def testmain = "Main" +} diff --git a/test/files/run/t4625.script b/test/files/run/t4625.script new file mode 100644 index 0000000000..600ceacbb6 --- /dev/null +++ b/test/files/run/t4625.script @@ -0,0 +1,5 @@ + +object Main extends Runnable with App { + def run() = println("Test ran.") + run() +} -- cgit v1.2.3 From 5bcefbe1889a1b8e1f9bac04090428eaaa7b2fd3 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 16 May 2016 22:57:51 -0700 Subject: SI-4625 App is a thing Scripting knows it by name. --- src/compiler/scala/tools/nsc/ast/parser/Parsers.scala | 5 ++++- src/reflect/scala/reflect/internal/StdNames.scala | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 7af5c505de..358ccb5dc3 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -365,12 +365,15 @@ self => val stmts = parseStats() def mainModuleName = newTermName(settings.script.value) + /* If there is only a single object template in the file and it has a * suitable main method, we will use it rather than building another object * around it. Since objects are loaded lazily the whole script would have * been a no-op, so we're not taking much liberty. */ def searchForMain(): Option[Tree] = { + import PartialFunction.cond + /* Have to be fairly liberal about what constitutes a main method since * nothing has been typed yet - for instance we can't assume the parameter * type will look exactly like "Array[String]" as it could have been renamed @@ -381,7 +384,7 @@ self => case _ => false } def isApp(t: Tree) = t match { - case Template(ps, _, _) => ps.exists { case Ident(x) if x.decoded == "App" => true ; case _ => false } + case Template(ps, _, _) => ps.exists(cond(_) { case Ident(tpnme.App) => true }) case _ => false } /* For now we require there only be one top level object. */ diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala index 52558d9395..a0688e129c 100644 --- a/src/reflect/scala/reflect/internal/StdNames.scala +++ b/src/reflect/scala/reflect/internal/StdNames.scala @@ -240,6 +240,7 @@ trait StdNames { final val Any: NameType = "Any" final val AnyVal: NameType = "AnyVal" + final val App: NameType = "App" final val FlagSet: NameType = "FlagSet" final val Mirror: NameType = "Mirror" final val Modifiers: NameType = "Modifiers" -- cgit v1.2.3 From ee365cccaf740d5ec353718556b010137f4cdd4d Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 16 May 2016 23:28:16 -0700 Subject: SI-4625 Permit arbitrary top-level in script In an unwrapped script, where a `main` entry point is discovered in a top-level object, retain all top-level classes. Everything winds up in the default package. --- src/compiler/scala/tools/nsc/ast/parser/Parsers.scala | 4 +++- test/files/run/t4625b.check | 1 + test/files/run/t4625b.scala | 7 +++++++ test/files/run/t4625b.script | 8 ++++++++ 4 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t4625b.check create mode 100644 test/files/run/t4625b.scala create mode 100644 test/files/run/t4625b.script diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 358ccb5dc3..1ece580b96 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -384,7 +384,7 @@ self => case _ => false } def isApp(t: Tree) = t match { - case Template(ps, _, _) => ps.exists(cond(_) { case Ident(tpnme.App) => true }) + case Template(parents, _, _) => parents.exists(cond(_) { case Ident(tpnme.App) => true }) case _ => false } /* For now we require there only be one top level object. */ @@ -402,6 +402,8 @@ self => */ if (name == mainModuleName) md else treeCopy.ModuleDef(md, mods, mainModuleName, template) + case md @ ModuleDef(_, _, _) => md + case cd @ ClassDef(_, _, _, _) => cd case _ => /* If we see anything but the above, fail. */ return None diff --git a/test/files/run/t4625b.check b/test/files/run/t4625b.check new file mode 100644 index 0000000000..e79539a5c4 --- /dev/null +++ b/test/files/run/t4625b.check @@ -0,0 +1 @@ +Misc top-level detritus diff --git a/test/files/run/t4625b.scala b/test/files/run/t4625b.scala new file mode 100644 index 0000000000..44f6225220 --- /dev/null +++ b/test/files/run/t4625b.scala @@ -0,0 +1,7 @@ + +import scala.tools.partest.ScriptTest + +object Test extends ScriptTest { + // must be called Main to get probing treatment in parser + override def testmain = "Main" +} diff --git a/test/files/run/t4625b.script b/test/files/run/t4625b.script new file mode 100644 index 0000000000..f21a553dd1 --- /dev/null +++ b/test/files/run/t4625b.script @@ -0,0 +1,8 @@ + +trait X { def x = "Misc top-level detritus" } + +object Bumpkus + +object Main extends X with App { + println(x) +} -- cgit v1.2.3 From e753135f02a8177e809937e56fed5c054091691f Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 17 May 2016 00:00:52 -0700 Subject: SI-4625 Warn when discarding script object It's pretty confusing when your script object becomes a local and then nothing happens. Such as when you're writing a test and it takes forever to figure out what's going on. --- src/compiler/scala/tools/nsc/ast/parser/Parsers.scala | 15 +++++++++++---- test/files/run/t4625c.check | 3 +++ test/files/run/t4625c.scala | 7 +++++++ test/files/run/t4625c.script | 6 ++++++ 4 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 test/files/run/t4625c.check create mode 100644 test/files/run/t4625c.scala create mode 100644 test/files/run/t4625c.script diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 1ece580b96..c2f2141fd3 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -389,8 +389,8 @@ self => } /* For now we require there only be one top level object. */ var seenModule = false + var disallowed = EmptyTree: Tree val newStmts = stmts collect { - case t @ Import(_, _) => t case md @ ModuleDef(mods, name, template) if !seenModule && (isApp(template) || md.exists(isMainMethod)) => seenModule = true /* This slightly hacky situation arises because we have no way to communicate @@ -404,11 +404,18 @@ self => else treeCopy.ModuleDef(md, mods, mainModuleName, template) case md @ ModuleDef(_, _, _) => md case cd @ ClassDef(_, _, _, _) => cd - case _ => + case t @ Import(_, _) => t + case t => /* If we see anything but the above, fail. */ - return None + disallowed = t + EmptyTree + } + if (disallowed.isEmpty) Some(makeEmptyPackage(0, newStmts)) + else { + if (seenModule) + warning(disallowed.pos.point, "Script has a main object but statement is disallowed") + None } - Some(makeEmptyPackage(0, newStmts)) } if (mainModuleName == newTermName(ScriptRunner.defaultScriptMain)) diff --git a/test/files/run/t4625c.check b/test/files/run/t4625c.check new file mode 100644 index 0000000000..6acb1710b9 --- /dev/null +++ b/test/files/run/t4625c.check @@ -0,0 +1,3 @@ +newSource1.scala:2: warning: Script has a main object but statement is disallowed +val x = "value x" + ^ diff --git a/test/files/run/t4625c.scala b/test/files/run/t4625c.scala new file mode 100644 index 0000000000..44f6225220 --- /dev/null +++ b/test/files/run/t4625c.scala @@ -0,0 +1,7 @@ + +import scala.tools.partest.ScriptTest + +object Test extends ScriptTest { + // must be called Main to get probing treatment in parser + override def testmain = "Main" +} diff --git a/test/files/run/t4625c.script b/test/files/run/t4625c.script new file mode 100644 index 0000000000..fa14f43950 --- /dev/null +++ b/test/files/run/t4625c.script @@ -0,0 +1,6 @@ + +val x = "value x" + +object Main extends App { + println(s"Test ran with $x.") +} -- cgit v1.2.3 From 7f514bba9ff1993ccbfdcf4a37a8045849f1647a Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 17 May 2016 14:12:57 -0700 Subject: SI-4625 Warn on first non-toplevel only Fixed the warning when main module is accompanied by snippets. Minor clean-up so even I can follow what is returned. --- .../scala/tools/nsc/ast/parser/Parsers.scala | 88 +++++++++++----------- test/files/run/t4625c.script | 1 + 2 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index c2f2141fd3..308669256d 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -371,7 +371,7 @@ self => * around it. Since objects are loaded lazily the whole script would have * been a no-op, so we're not taking much liberty. */ - def searchForMain(): Option[Tree] = { + def searchForMain(): Tree = { import PartialFunction.cond /* Have to be fairly liberal about what constitutes a main method since @@ -387,10 +387,10 @@ self => case Template(parents, _, _) => parents.exists(cond(_) { case Ident(tpnme.App) => true }) case _ => false } - /* For now we require there only be one top level object. */ + /* We allow only one main module. */ var seenModule = false var disallowed = EmptyTree: Tree - val newStmts = stmts collect { + val newStmts = stmts.map { case md @ ModuleDef(mods, name, template) if !seenModule && (isApp(template) || md.exists(isMainMethod)) => seenModule = true /* This slightly hacky situation arises because we have no way to communicate @@ -407,54 +407,58 @@ self => case t @ Import(_, _) => t case t => /* If we see anything but the above, fail. */ - disallowed = t + if (disallowed.isEmpty) disallowed = t EmptyTree } - if (disallowed.isEmpty) Some(makeEmptyPackage(0, newStmts)) + if (disallowed.isEmpty) makeEmptyPackage(0, newStmts) else { if (seenModule) warning(disallowed.pos.point, "Script has a main object but statement is disallowed") - None + EmptyTree } } - if (mainModuleName == newTermName(ScriptRunner.defaultScriptMain)) - searchForMain() foreach { return _ } + def mainModule: Tree = + if (mainModuleName == newTermName(ScriptRunner.defaultScriptMain)) searchForMain() else EmptyTree - /* Here we are building an AST representing the following source fiction, - * where `moduleName` is from -Xscript (defaults to "Main") and are - * the result of parsing the script file. - * - * {{{ - * object moduleName { - * def main(args: Array[String]): Unit = - * new AnyRef { - * stmts - * } - * } - * }}} - */ - def emptyInit = DefDef( - NoMods, - nme.CONSTRUCTOR, - Nil, - ListOfNil, - TypeTree(), - Block(List(Apply(gen.mkSuperInitCall, Nil)), literalUnit) - ) - - // def main - def mainParamType = AppliedTypeTree(Ident(tpnme.Array), List(Ident(tpnme.String))) - def mainParameter = List(ValDef(Modifiers(Flags.PARAM), nme.args, mainParamType, EmptyTree)) - def mainDef = DefDef(NoMods, nme.main, Nil, List(mainParameter), scalaDot(tpnme.Unit), gen.mkAnonymousNew(stmts)) - - // object Main - def moduleName = newTermName(ScriptRunner scriptMain settings) - def moduleBody = Template(atInPos(scalaAnyRefConstr) :: Nil, noSelfType, List(emptyInit, mainDef)) - def moduleDef = ModuleDef(NoMods, moduleName, moduleBody) - - // package { ... } - makeEmptyPackage(0, moduleDef :: Nil) + def repackaged: Tree = { + /* Here we are building an AST representing the following source fiction, + * where `moduleName` is from -Xscript (defaults to "Main") and are + * the result of parsing the script file. + * + * {{{ + * object moduleName { + * def main(args: Array[String]): Unit = + * new AnyRef { + * stmts + * } + * } + * }}} + */ + def emptyInit = DefDef( + NoMods, + nme.CONSTRUCTOR, + Nil, + ListOfNil, + TypeTree(), + Block(List(Apply(gen.mkSuperInitCall, Nil)), literalUnit) + ) + + // def main + def mainParamType = AppliedTypeTree(Ident(tpnme.Array), List(Ident(tpnme.String))) + def mainParameter = List(ValDef(Modifiers(Flags.PARAM), nme.args, mainParamType, EmptyTree)) + def mainDef = DefDef(NoMods, nme.main, Nil, List(mainParameter), scalaDot(tpnme.Unit), gen.mkAnonymousNew(stmts)) + + // object Main + def moduleName = newTermName(ScriptRunner scriptMain settings) + def moduleBody = Template(atInPos(scalaAnyRefConstr) :: Nil, noSelfType, List(emptyInit, mainDef)) + def moduleDef = ModuleDef(NoMods, moduleName, moduleBody) + + // package { ... } + makeEmptyPackage(0, moduleDef :: Nil) + } + + mainModule orElse repackaged } /* --------------- PLACEHOLDERS ------------------------------------------- */ diff --git a/test/files/run/t4625c.script b/test/files/run/t4625c.script index fa14f43950..16159208e0 100644 --- a/test/files/run/t4625c.script +++ b/test/files/run/t4625c.script @@ -1,5 +1,6 @@ val x = "value x" +val y = "value y" object Main extends App { println(s"Test ran with $x.") -- cgit v1.2.3