Invoke all Enso benchmarks via JMH #7101

Akirathan · 2023-06-22T09:33:29Z

Pull Request Description

Motivation

There are a lot of Enso-only benchmarks in test/Benchmarks project. These benchmarks are not run on the CI (they are, but only in dry-run). We want to run these benchmarks and collect the results, such that we will see them in engine-benchmark-results along with all the other Engine bench results. These benchmarks use our Enso benchmarking infrastrcture, which basically consist of just one method - Bench.measure.

In order to do that, we have two choices:

Try to invoke everything via JMH, so that JMH will provide all the data processing and data collection.
Implement result collector to our benchmarking infrastructure.

Let's invoke every benchmark via JMH. Not only it collects the results for us, but it also does a lot of additional work like JVM preparation, forking, warmup, etc. We don't want to lose that ability.

Ideal properties of the solution:

We don't want to manually rewrite all the benchmark sources.
- Ideally, there is no need to any modification to any of these sources.
We still want to be able to invoke the benchmarks as standalone scripts, just as we can do right now.

How to use it

This PR adds two new SBT projects - bench-processor in libs/scala/bench-processor directory, and std-benchmarks in std-bits/benchmarks directory. std-benchmarks has just one Java class with the main method and with @GenerateBenchSources annotation.

To run one benchmark:

std-benchmarks/benchOnly <bench-name-regex>

To force the annotation processor to rediscover new benchmarks and to regenerate the JMH sources:

std-benchmarks/clean; std-benchmarks/Bench/clean; std-benchmarks/Bench/compile

To provide additional cmdline arguments that are supported by JMH runner:

std-benchmarks/Bench/run -h

More info in docs/infrastructure/benchmarks.md

Important Notes

The Plot

there used to be two kinds of benchmarks: in Java and in Enso
those in Java got quite a good treatment
there even are results updated daily: https://enso-org.github.io/engine-benchmark-results/
the benchmarks written in Enso used to be 2nd class citizen

The Revelation

This PR has the potential to fix it all!

It designs new Bench API ready for non-batch execution
It allows for single benchmark in a dedicated JVM execution
It provides a simple way to wrap such an Enso benchmark as a Java benchmark
thus the results of Enso and Java benchmarks are now unified

Long live single benchmarking infrastructure for Java and Enso!

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

...untime/src/bench/resources/org.enso.interpreter.bench.benchmarks.meso/Vector_Operations.enso

hubertp · 2023-06-23T15:31:45Z

@Akirathan could we also combine this with #6554?

Akirathan · 2023-06-23T17:17:51Z

@Akirathan could we also combine this with #6554?

Well, this kind of supersedes #6554 - if this is integrated, there will be no more need for Enso benchmarks test/Benchmarks to provide any sophisticated output because both Engine benchmarks and Enso benchmarks will be run via JMH.

engine/runtime/src/bench/scala/org/enso/interpreter/bench/IRUtils.scala

… them

test/Benchmarks/src/Vector/Operations.enso

distribution/lib/Standard/Test/0.0.0-dev/src/Bench.enso

Akirathan · 2023-07-07T15:53:55Z

Current idea of the solution (sketched in f6dd423):

There will be one class LibBenchRunner with main method that:
- Collects all the benchmark specifications from test/Benchmarks project
- Parses JMH specific cmdline arguments and merges them with some custom arguments
  - Inspired by Main.java from JMH project that parses all the arguments.
- Delegates to a proper benchmark
One class LibBench with a single method annotated with @Benchmark.
- Needed in order for the JMH runner to recognize it as a benchmark.
- This method takes parameters and extracts the benchmark name from it and delegates to an appropriate benchmark

Another alternative is to generate many Java sources with many methods annotated with @Benchmark based on discovered benchmarks from test/Benchmarks, but that is too complicated.

IR manipulation is out of the question - better to refactor test/Benchmarks than manipulate with IR.

This idea assumes that all the benchmarks in test/Benchmarks are migrated to the builder pattern.

engine/runtime/src/bench/java/org/enso/interpreter/bench/benchmarks/meso/LibBenchRunner.java

…-bench-jmh

project/FrgaalJavaCompiler.scala

JaroslavTulach

I've just used this support to execute Greg's benchmarks and see the results in IGV. I think the support is good enough.

docs/CONTRIBUTING.md

radeusgd · 2023-08-03T15:26:00Z

docs/debugger/chrome-devtools.md

+- Use `env JAVA_OPTS=-Dpolyglot.inspect.Path=enso_debug` to set the chrome to
+  use a fixed URL. In this case the URL is
+  `devtools://devtools/bundled/js_app.html?ws=127.0.0.1:9229/enso_debug`


Wow I did not know that, so cool!

docs/infrastructure/benchmarks.md

radeusgd · 2023-08-03T15:34:32Z

docs/infrastructure/benchmarks.md

+
+The `std-benchmarks` SBT project supports `bench` and `benchOnly` commands, that
+work the same as in the `runtime` project, with the exception that the benchmark
+name does not have to be specified as a fully qualified name, but as a regular


Does this not work in runtime?

(using regexp to filter benchmarks)

Not yet, I will look into that in one of the follow-up PRs and unify the functionality even more.

docs/infrastructure/benchmarks.md

lib/scala/bench-processor/src/main/java/org/enso/benchmarks/BenchConfig.java

radeusgd · 2023-08-03T15:39:53Z

lib/scala/bench-processor/src/main/java/org/enso/benchmarks/BenchSpec.java

+ */
+public interface BenchSpec {
+  String name();
+  Value code();


Is it really the code or more like an executable thunk?

It's exactly the same as code field in Bench.Spec, as mentioned in the javadoc. It's just a polyglot org.graalvm.polyglot.Value that can be executed. I guess that technically it is a thunk, but what difference does it make here?

radeusgd · 2023-08-03T15:50:17Z

project/FrgaalJavaCompiler.scala

+      if (debugAnotProcessorOpt) {
+        log.info(
+          s"Frgaal compiler is about to be launched with $debugArg, which means that" +
+          " it will wait for a debugger to attach. The output from the compiler is by default" +
+          " redirected, therefore \"Listening to the debugger\" message will not be displayed." +
+          " You should attach the debugger now."
+        )
+      }


Wow, thanks for adding this message!

Without that message, you would have no idea that the java process is waiting for a debugger, because the output is redirected.

radeusgd · 2023-08-03T15:57:59Z

test/Benchmarks/src/Vector/Distinct.enso

+random_vec = Utils.make_random_vec 100000
+uniform_vec = Vector.fill 100000 1
+random_text_vec = random_vec.map .to_text
+uniform_text_vec = random_vec.map .to_text


This is not good.

Enso has a subtle difference between top-level and scoped definitions, that plays a very crucial role here.

A scoped definition is computed (unless it's a block...) and stored. A top-level definition is re-computed on every access.

Essentially you have turned values computed once at initialization of the suite into 0-argument methods that are recomputed on each access.

This changes the semantics of these benchmarks.

We used to be just computing the time needed to compute the distinct operation. Now we are computing the total time it takes to both generate the vector and compute distinct.

I don't think that's right.

I know that we don't have the 'setup' pattern yet. For now I'd just run this inside of the group. It's not perfect and will slow down gathering benchmarks, so maybe for future we need a better solution.

On the other hand, I'm not 100% convinced that changing semantics is bad here. I don't think it is good but feel free to convince me otherwise.

A scoped definition is computed (unless it's a block...) and stored. A top-level definition is re-computed on every access.

I have not realized that. That might be problematic here - we don't want a different random_vec in each iteration. I will try to revert that. My goal here was just to demonstrate the usage of the builder pattern that is syntactically a bit nicer than the usage in Operations.enso.

On second thought, let me take care of this in follow-up PRs, I have now added it to a task list in #7489.

Akirathan · 2023-08-07T10:16:06Z

engine/runtime/src/bench/java/org/enso/interpreter/bench/BenchmarksRunner.java

@@ -39,7 +41,8 @@ public BenchmarkItem run(String label) throws RunnerException, JAXBException {
    if (Boolean.getBoolean("bench.compileOnly")) {
      builder
        .measurementIterations(1)
-        .warmupIterations(0);
+        .warmupIterations(0)
+        .forks(0);


Setting forks to 0 when bench.CompileOnly = true seems to have a negative side effect when all the benchmarks are run on the CI - it seems to cause StackOverflow errors, like this one

I have reverted 11e0a12 as a workaround for now. In the upcoming PRs, I will try to unify and improve the debugging experience of the benchmarks. Tracking this in #7489.

This reverts commit 95e6dea.

JaroslavTulach · 2023-08-10T11:21:05Z

project/FrgaalJavaCompiler.scala

@@ -29,6 +29,9 @@ object FrgaalJavaCompiler {
  val frgaal      = "org.frgaal" % "compiler" % "19.0.1" % "provided"
  val sourceLevel = "19"

+  val debugArg =
+    "-J-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=localhost:8000"


I believe this option shall be unified with WithDebugCommand setup... I wanted to mention that before, but maybe I haven't done so...

You have mentioned that already. And the answer is buried somewhere within dozens of comments in this PR. The brief answer is that we need the javac process (more specifically, in this case, java -jar frgaal-compiler.jar process) to wait for the debugger and not to start without it. That is because the java -jar frgaal-compiler.jar process's output is piped, so the compiler process can finish even before the user notices that they could attach a debugger.

Moreover, using WithDebugCommand.DEBUG_ARG requires some more refactoring - for that, we would need to remove some files in project directory from org.enso.build package.

TL;DR; it would be unnecessarily complicated.

Akirathan commented Jun 22, 2023

View reviewed changes

...untime/src/bench/resources/org.enso.interpreter.bench.benchmarks.meso/Vector_Operations.enso Outdated Show resolved Hide resolved

[WIP] Invoke Enso bench via JMH

188d465

Akirathan force-pushed the wip/akirathan/enso-bench-jmh branch from 39fc4c9 to 188d465 Compare June 26, 2023 17:01

JaroslavTulach reviewed Jun 30, 2023

View reviewed changes

engine/runtime/src/bench/scala/org/enso/interpreter/bench/IRUtils.scala Outdated Show resolved Hide resolved

JaroslavTulach added 2 commits July 1, 2023 08:57

Abstract from direct call to Bench.measure

8836629

Builder pattern to collect all benchmarks first and only then execute…

88fd6fb

… them

JaroslavTulach reviewed Jul 1, 2023

View reviewed changes

test/Benchmarks/src/Vector/Operations.enso Show resolved Hide resolved

JaroslavTulach reviewed Jul 1, 2023

View reviewed changes

distribution/lib/Standard/Test/0.0.0-dev/src/Bench.enso Outdated Show resolved Hide resolved

JaroslavTulach reviewed Jul 1, 2023

View reviewed changes

distribution/lib/Standard/Test/0.0.0-dev/src/Bench.enso Outdated Show resolved Hide resolved

enso-bot bot mentioned this pull request Jul 1, 2023

Recalculate only necessary nodes when a node changes #7020

Closed

Akirathan added 2 commits July 7, 2023 17:44

Improve type ascriptions of Bench type

19a439e

[WIP] Add LibBenchRunner

f6dd423

JaroslavTulach reviewed Jul 9, 2023

View reviewed changes

engine/runtime/src/bench/java/org/enso/interpreter/bench/benchmarks/meso/LibBenchRunner.java Outdated Show resolved Hide resolved

JaroslavTulach added 2 commits July 9, 2023 17:39

Making the project compilable (again)

58c1e0d

Executed few suites from Vector/Operations.enso in JMH

f5a9aae

enso-bot bot mentioned this pull request Jul 10, 2023

Dataflow error in a default of a type-ascribed argument results in a type error instead of being propagated #7137

Closed

JaroslavTulach assigned Akirathan Jul 10, 2023

JaroslavTulach mentioned this pull request Jul 10, 2023

Calculating sum is 10x slower using Statistics #5067

Closed

JaroslavTulach added 4 commits July 10, 2023 14:22

Merge remote-tracking branch 'origin/develop' into wip/akirathan/enso…

3f37a7d

…-bench-jmh

Bench is the type, not Group

7c33b3a

sumStatistic benchmark

f70a6da

Generate double values

1584357

JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Jul 11, 2023

JaroslavTulach mentioned this pull request Jul 13, 2023

stdlib benchmarks should produce reports in a unified format #6554

Closed

radeusgd linked an issue Jul 13, 2023 that may be closed by this pull request

stdlib benchmarks should produce reports in a unified format #6554

Closed

Akirathan added 2 commits July 17, 2023 17:45

Add prototype of bench suites discovery

0f41777

Move LibBenchRunner to a separate project bench-libs

9d9f328

JaroslavTulach reviewed Aug 3, 2023

View reviewed changes

project/FrgaalJavaCompiler.scala Outdated Show resolved Hide resolved

Akirathan marked this pull request as ready for review August 3, 2023 13:22

Akirathan requested review from GregoryTravis, 4e6 and hubertp as code owners August 3, 2023 13:22

Remove --no-limit-modules

2664ded

Akirathan mentioned this pull request Aug 3, 2023

Ability to invoke all std-benchmarks via JMH #7489

Closed

fmt docs

08fa60b

JaroslavTulach approved these changes Aug 3, 2023

View reviewed changes