Address community feedback #27

xeno-by · 2017-11-24T02:54:26Z

This pull request adds a performance comparison between Rsc schedule and Scalac namer to sbt bench (#25) and also refactors the benchmark suite to support both Scala 2.11 and Scala 2.12 (#26).

Todo:

Update docs/compiler.md with a new section that provides a detailed description of similarities and differences between Rsc and Scalac pipelines.
Link the disclaimer in docs/performance.md to the aforementioned new section in documentation.
Update docs/performance.md with new performance numbers once our benchmark box finishes running the updated benchmarks.

/cc @dragos @adriaanm @retronym

CLAassistant · 2017-11-24T02:54:32Z

All committers have signed the CLA.

xeno-by · 2017-11-24T03:14:43Z

build.sbt

  )
+
+lazy val benchScalac212 = project


@eed3si9n @dwijnand Can you provide some advice on deduplicating code in benchScalac211 and benchScalac212? Files in these two projects are currently completely identical, which is clearly suboptimal, but I don't know how to make sbt do my bidding otherwise.

My goal is to be able to say benchScalac211/jmh:run ... or benchScalac212/jmh:run ... and then have sbt run the same benchmarks for either Scala 2.11 or Scala 2.12.

Here's what I tried so far:

One project in conjunction with ++. This works, but I don't like the fact that users will have to know exact Scala versions (e.g. 2.12.4 vs simply 2.12) to run benchmarks. Even worse, if the user forgets to say ++, they may inadvertently run benchmarks for an unexpected version Scala. I think this is a big deal, because benchmarking is monotonous enough to make humans accidentally miss things like that.

Two projects that are rooted at the same directory. This seems to be illegal.

Three projects (a common project that crosscompiles to Scala 2.11 and Scala 2.12, as well as two projects that depend on it). This leads to an error similar to the one described at https://stackoverflow.com/questions/27929272/how-to-have-sbt-subproject-with-multiple-scala-versions.

It’s a bit hacky but I think you can use separate empty project directories and set them to have the same managed sources (or additional sources) to get around the fact that two projects can’t have the same directory.

Interesting idea, thanks! Let's keep this in mind for the future.

Sorry, with all the reasonable-scala repo activity I missed this notification was directly at me.

I too would go the way of adding a shared source directory to both projects.

xeno-by · 2017-11-24T03:24:02Z

bench/scalac212/src/main/scala/rsc/bench/ScalacCompile.scala

+  def run(bs: BenchmarkState): Unit = {
+    runImpl(bs)
+  }
+}


@retronym I've been meaning to compare notes wrt. scala/compiler-benchmark right after we opensourced Rsc, but forgot about that in the post-release rush. Looks like we have another good opportunity to chat.

Just like you guys, we found it useful to split benchmarks into cold, warm and hot. In our benchmarks, cold and hot cover two important use cases - single-shot CLI runs and resident compilations. Moreover, we use warm to get a quick & dirty approximation of hot during interactive development.

Unfortunately, because of the peculiar configuration style of JMH, we haven't found a way to abstract these notions, so we have to create endless bunches of ColdXxxYyy, WarmXxxYyy and HotXxxYyy classes with the same JMH annotations repeated over and over again.

I guess you guys faced the same problem in scala/compiler-benchmark. Do you maybe have some ideas how to cut down on duplication?

ShaneDelmore · 2017-11-24T06:30:55Z

build.sbt

  .enablePlugins(JmhPlugin)
+  .jvmSettings(benchCliRscNative("Typecheck"))


You can resolve #22 in the pr while you're at it by adding:

.nativeSettings(nativeMode := "release)

This commit splits project `bench` into multiple projects: `benchJavac18`, `benchRsc`, `benchScalac211` and `benchScalac212`. This provides additional flexibility and fine-grained control over target compiler versions. The main motivation for this change is the fact that neither Scala 2.11 nor Scala 2.12 clearly dominate one another on re2s. Scala 2.11 is faster in cold mode, whereas Scala 2.12 is faster in hot mode. While refactoring the benchmarking infrastructure to accommodate this change, I've also retired CliBench. It is confusing to have multiple benchmarks that test the same thing (ColdXxxYyy and CliXxxYyy), so I picked the one that's simpler, i.e. ColdXxxYyy powered by JMH. RscNativeTypecheck is still powered by CliBench-like logic, but it has been merged directly into that particular benchmark. I am not completely happy with the astonishing degree of duplication between benchScalac211 and benchScalac212, but my sbt skills don't allow me to remove this duplication in a satisfying manner. (I'm aware of ++, but I want Scala versions to be part of the project name, not an optional prefix to an sbt invocation.)

This will make it easier to interpret benchmark results - both via eyeballing and via automated processing.

> benchRscJVM/benchRscNativeSchedule [info] Linking (1053 ms) [info] Discovered 2088 classes and 15212 methods [info] Optimizing (1377 ms) [info] Generating intermediate code (418 ms) [info] Produced 54 files [info] Compiling to native code (1266 ms) [info] Linking native code (205 ms) [info] Running rsc.bench.RscNativeSchedule ~/Projects/rsc/bench/rsc/native/target/scala-2.11/benchrscnative-out ... Result "rsc.bench.ColdRscNativeSchedule.run": N = 100 mean = 146.522 ms/op Benchmark Mode Cnt Score Units ColdRscNativeSchedule.run cli 100 146.522 ms/op ... Result "rsc.bench.HotRscNativeSchedule.run": N = 1 mean = 119.297 ms/op Benchmark Mode Cnt Score Units HotRscNativeSchedule.run cli 1 119.297 ms/op [success] Total time: 32 s, completed Nov 24, 2017 2:23:06 PM

Concretely: * ScalacScanNnn => ScalacScannerNnn * ScalacParseNnn => ScalacParserNnn * ScalacNameNnn => ScalacNamerNnn * ScalacTypecheckNnn => ScalacTyperNnn These names break consistency with the corresponding Rsc benchmarks, but make it easy to correlate what's going on with the corresponding Scalac modules.

ShaneDelmore

Nice PR. I especially appreciated the script to update performance.md.

xeno-by added this to the 0.1.0 "Bare minimum" milestone Nov 24, 2017

xeno-by mentioned this pull request Nov 24, 2017

Add performance comparison against Scala namer. #23

Closed

xeno-by commented Nov 24, 2017

View reviewed changes

ShaneDelmore reviewed Nov 24, 2017

View reviewed changes

xeno-by added 15 commits November 24, 2017 17:03

Add Rsc schedule and Scalac namer to sbt bench

f579efd

Update the branch naming policy in contributing.md

b997f68

Update performance.md after rerunning benchmarks

b95a93c

Fix a typo in javac benchmark configuration

e1912fc

Rename ScalacXxx to ScalacXxx211 and ScalacXxx212

51c723f

This will make it easier to interpret benchmark results - both via eyeballing and via automated processing.

Add the RscNativeSchedule benchmark

2854b54

Add a script that automatically updates performance.md

f6bfe3b

Fix HTML markup in performance.md

22e4c57

Summarize similarities and differences between Rsc and Scalac

24dde70

Revive the "Comments" section in performance.md

26100d9

Use release mode of Scala Native for benchmarks (fix twitter#22)

263273d

Update performance.md after rerunning benchmarks

5632ff7

xeno-by force-pushed the community-feedback branch from 69cbdf5 to 5632ff7 Compare November 25, 2017 01:05

xeno-by added 5 commits November 24, 2017 21:31

Speed up testsNative

d377944

Also aggregate rscNative and testsNative

6c8397b

Add convenience aliases for running subsets of the benchmark suite

df30954

Fix accidentally hardcoded values in update_documentation

0a68aca

Add more emphasis on Rsc currently being a prototype

827d521

ShaneDelmore previously approved these changes Nov 25, 2017

View reviewed changes

Explain how to work around IntelliJ issues

199860f

xeno-by dismissed ShaneDelmore’s stale review via 199860f November 25, 2017 18:17

ShaneDelmore approved these changes Nov 25, 2017

View reviewed changes

ShaneDelmore merged commit 85f54b1 into twitter:master Nov 25, 2017

This was referenced Nov 25, 2017

Add performance comparison between Rsc schedule and Scalac namer #25

Closed

Fix Intellij highlight errors #13

Closed

xeno-by deleted the community-feedback branch November 25, 2017 18:34

xeno-by added the Docs label Jul 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address community feedback #27

Address community feedback #27

xeno-by commented Nov 24, 2017 •

edited

Loading

CLAassistant commented Nov 24, 2017 •

edited

Loading

xeno-by Nov 24, 2017

ShaneDelmore Nov 25, 2017

xeno-by Nov 25, 2017

dwijnand Nov 28, 2017

xeno-by Nov 24, 2017

ShaneDelmore Nov 24, 2017

xeno-by Nov 25, 2017

ShaneDelmore left a comment

		.enablePlugins(JmhPlugin)
		.jvmSettings(benchCliRscNative("Typecheck"))

Address community feedback #27

Address community feedback #27

Conversation

xeno-by commented Nov 24, 2017 • edited Loading

CLAassistant commented Nov 24, 2017 • edited Loading

xeno-by Nov 24, 2017

Choose a reason for hiding this comment

ShaneDelmore Nov 25, 2017

Choose a reason for hiding this comment

xeno-by Nov 25, 2017

Choose a reason for hiding this comment

dwijnand Nov 28, 2017

Choose a reason for hiding this comment

xeno-by Nov 24, 2017

Choose a reason for hiding this comment

ShaneDelmore Nov 24, 2017

Choose a reason for hiding this comment

xeno-by Nov 25, 2017

Choose a reason for hiding this comment

ShaneDelmore left a comment

Choose a reason for hiding this comment

xeno-by commented Nov 24, 2017 •

edited

Loading

CLAassistant commented Nov 24, 2017 •

edited

Loading