Add wasm_of_ocaml benchmarks with current-bench output #1842

OlivierNicole · 2025-02-24T17:06:17Z

@vouillon and I (so far, mostly @vouillon) have started to look at ways of improving performance of Wasm code. To keep track of our progress, we need to set up a benchmarking suite that gets run regularly.

This PR makes the repository compatible for monitoring by https://github.com/ocurrent/current-bench/: the benchmark suite is extended with the current-bench output format and I modified the scripts so that Wasm benchmarks are first-class citizens. The existing benchmarking utilities should still work, I ran a few checks to verify that.

This is a first step, we will soon want to add macrobenchmarks as well — presumably by having a benchmarks/sources/macro directory where each subdirectory is a macrobenchmark with its own dune file.

OlivierNicole · 2025-02-24T17:08:23Z

Note: this is ready for review but I still need to verify details—namely, how current-bench deals with multiple opam files, and what should be the name of the results file—so we should hold off from merging yet.

OlivierNicole · 2025-02-27T14:15:40Z

Thanks to @punchagan’s advice, this should now hopefully work with current-bench.

vouillon · 2025-02-27T14:49:04Z

I think make bench needs to build wasm_of_ocaml. The opam file is here only to install external dependencies.

vouillon · 2025-02-27T15:02:42Z

I would put all the results together and grouping them?

{
  "name": "Wasm_of_ocaml",
  "results": [
    {
      "name": "Microbenchmarks",
      "metrics": [
        {
          "name": "microbenchmark/almabench",
          "value": 1.6087265,
          "units": "s"
        },
        {
          "name": "microbenchmark/bdd",
          "value": 0.31883700000000004,
          "units": "s"
        },

OlivierNicole · 2025-02-27T15:14:32Z

This will cause all the run times to appear on one unique graph. I’m not sure we want that?

wasm_of_ocaml-compiler-bench.opam

vouillon · 2025-02-27T16:24:43Z

This will cause all the run times to appear on one unique graph. I’m not sure we want that?

Maybe not, actually, since they have very different running times. This is done for Ocaml, but it is not very readable.

We should still put them in the same list of metrics. This will be more readable when we will add other benchmarks.

Maybe we could report the geometric mean of all the running times as well?

OlivierNicole · 2025-02-28T10:18:26Z

I think make bench needs to build wasm_of_ocaml. The opam file is here only to install external dependencies.

Actually, I disagree. When you look at the last line of https://github.com/ocurrent/current-bench/blob/b0528d005b34b8ea1ff4cd7f893165ce782ef5db/pipeline/lib/custom_dockerfile.ml#L78-L96, it also installs regular dependencies.

I have rearranged the metrics list as requested.

Maybe we could report the geometric mean of all the running times as well?

A geometric mean of absolute values? Isn’t the geometric mean usually used on relative evolution numbers?

OlivierNicole · 2025-02-28T10:26:06Z

This repo has been added to the current-bench infra and the results for this PR can be checked here.

bench.Dockerfile

This reverts commit 8556727.

OlivierNicole · 2025-03-05T18:21:54Z

I have opened #1854 which should unblock the installation of Binaryen as a binary here.

This reverts commit 4a495e9.

hhugo · 2025-03-06T10:14:41Z

benchmarks/Makefile

+# This target is the one run by `current-bench`,
+# see https://github.com/ocurrent/current-bench
+.PHONY: bench
+bench: __run_wasm


Shouldn't this be called bench-wasm ?

hhugo · 2025-03-06T10:22:53Z

benchmarks/run.ml

+  if js
+  then (
+    compile_jsoo ~wasm "" code Spec.byte code Spec.js_of_ocaml;


This doesn't look correct. if both js and wasm are set, this will use wasm_of_ocaml instead of js_of_ocaml to compile the file.
I think it would be better to rename compile_jsoo with something like

val compile_js : backend:[`Jsoo | `Wasmoo] -> ...

hhugo · 2025-03-06T10:29:33Z

benchmarks/run.ml

  compile "ocamlc -unsafe" src Spec.ml code Spec.byte_unsafe;
  compile "ocamlopt" src Spec.ml code Spec.opt_unsafe;
-  compile_jsoo "" code Spec.byte_unsafe code Spec.js_of_ocaml_unsafe;
+  if wasm then compile_jsoo ~wasm "" code Spec.byte code Spec.wasm_of_ocaml;


Why don't we take effects into account ?

hhugo · 2025-03-06T10:43:02Z

benchmarks/run.ml

-        ; Some Spec.js_of_ocaml_call
-        ; Some Spec.js_of_ocaml_effects
-        ] )
-    else if effects


You've dropped this logic. The documentation of effects was previously:

only run with and without effect handler support

which I think meant that we wanted to run a small subset of the runs.

hhugo · 2025-03-06T10:46:54Z

benchmarks/run.ml

-    ; "-effects", Arg.Set effects, " only run with and without effect handler support"
+    ; ( "-effects"
+      , Arg.Symbol
+          ( [ "none"; "cps"; "double-translation" ]
+          , function
+            | "none" -> set_effects `None
+            | "cps" -> set_effects `Cps
+            | "double-translation" -> set_effects `Double_translation
+            | _ -> assert false )
+      , " only run with and without effect handler support" )


I'm not sure this is what we want. When benching for effects, we probably when to be able to compare all backends.

hhugo

The PR does many things at once

Setup docker LGTM
Some refactoring of the benchmarks (e.g introducing Measure). Can we move that to a separate PR ?
Add a new output format. Can we move that to a separate PR (or together with the refactoring above)
Add support for effects backend. I don't think it is correct. We probably want to be able to bench different backends / bench all backend or none ? Can this be moved to a different PR
Add support for wasm in benchmarks. Can we have this in a dedicated PR ?

Maybe we could have the following stack of PR

refactoring and new output format
wasm support and docker setup
effects backends

The benchmark scripts have received very little attention so far. Maybe it's a good time for larger refactoring.

run.ml is responsible for producing all data for reports. It has a few flags to produce a subset of data, presumably to speedup the process.
We could image a way to only generate the data we need for a set of reports.

hhugo · 2025-03-06T12:14:14Z

See #1855

hhugo · 2025-03-06T12:26:54Z

I think it would be useful to extend the scope of this to track things like

run time for jsoo files
build time for large executable (ocamlc ? / jsoo-toplevel ?) with jsoo and wasmoo
memory consumption when building theses larges executables

OlivierNicole · 2025-03-06T16:52:06Z

Superseded by #1858 and #1860.

OlivierNicole marked this pull request as ready for review February 24, 2025 17:06

OlivierNicole force-pushed the wasm-benchmarks branch from 458a34c to df5a387 Compare February 27, 2025 14:16

vouillon reviewed Feb 27, 2025

View reviewed changes

wasm_of_ocaml-compiler-bench.opam Outdated Show resolved Hide resolved

OlivierNicole force-pushed the wasm-benchmarks branch 15 times, most recently from 9e15f25 to 9087508 Compare March 4, 2025 13:25

OlivierNicole mentioned this pull request Mar 4, 2025

Stop using dune subst? #1852

Closed

OlivierNicole force-pushed the wasm-benchmarks branch 2 times, most recently from 2846158 to da6c377 Compare March 4, 2025 14:35

hhugo reviewed Mar 4, 2025

View reviewed changes

bench.Dockerfile Outdated Show resolved Hide resolved

OlivierNicole added 12 commits March 5, 2025 16:37

Add changelog entry

9b37494

Fix up setup

3af2f30

CR: Install wasm_of_ocaml before running benchmarks

510319a

Fix dune-project and rearrange metrics

144814e

reformat

52b4db5

Revert "CR: Install wasm_of_ocaml before running benchmarks"

8eda47c

This reverts commit 8556727.

Fix dune build

309ac19

Add bench.Dockerfile

88f1313

Do more runs

ddf794e

Remove unnecessary opam file, improve Dockerfile

6e3fd6b

Remove unnecessary command

8c85774

Remove dune subst git hack after ocsigen#1853

4262355

OlivierNicole force-pushed the wasm-benchmarks branch from 9913438 to 4262355 Compare March 5, 2025 15:56

OlivierNicole added 2 commits March 5, 2025 19:03

Remove dependency on binaryen-bin

4a495e9

Install binaryen in Dockerfile

129d9a5

OlivierNicole mentioned this pull request Mar 5, 2025

Remove dependency of wasm_of_ocaml on binaryen-bin #1854

Closed

hhugo and others added 2 commits March 6, 2025 10:49

Simplify docker file

c05923d

Revert "Remove dependency on binaryen-bin"

cfb6938

This reverts commit 4a495e9.

hhugo reviewed Mar 6, 2025

View reviewed changes

OlivierNicole mentioned this pull request Mar 6, 2025

Refactor benchmark script and support current-bench #1858

Merged

OlivierNicole closed this Mar 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wasm_of_ocaml benchmarks with current-bench output #1842

Add wasm_of_ocaml benchmarks with current-bench output #1842

OlivierNicole commented Feb 24, 2025

OlivierNicole commented Feb 24, 2025

OlivierNicole commented Feb 27, 2025

vouillon commented Feb 27, 2025

vouillon commented Feb 27, 2025

OlivierNicole commented Feb 27, 2025

vouillon commented Feb 27, 2025

OlivierNicole commented Feb 28, 2025

OlivierNicole commented Feb 28, 2025

OlivierNicole commented Mar 5, 2025

hhugo Mar 6, 2025

hhugo Mar 6, 2025

hhugo Mar 6, 2025

hhugo Mar 6, 2025

hhugo Mar 6, 2025

hhugo left a comment

hhugo commented Mar 6, 2025

hhugo commented Mar 6, 2025

OlivierNicole commented Mar 6, 2025

Add wasm_of_ocaml benchmarks with current-bench output #1842

Add wasm_of_ocaml benchmarks with current-bench output #1842

Conversation

OlivierNicole commented Feb 24, 2025

OlivierNicole commented Feb 24, 2025

OlivierNicole commented Feb 27, 2025

vouillon commented Feb 27, 2025

vouillon commented Feb 27, 2025

OlivierNicole commented Feb 27, 2025

vouillon commented Feb 27, 2025

OlivierNicole commented Feb 28, 2025

OlivierNicole commented Feb 28, 2025

OlivierNicole commented Mar 5, 2025

hhugo Mar 6, 2025

Choose a reason for hiding this comment

hhugo Mar 6, 2025

Choose a reason for hiding this comment

hhugo Mar 6, 2025

Choose a reason for hiding this comment

hhugo Mar 6, 2025

Choose a reason for hiding this comment

hhugo Mar 6, 2025

Choose a reason for hiding this comment

hhugo left a comment

Choose a reason for hiding this comment

hhugo commented Mar 6, 2025

hhugo commented Mar 6, 2025

OlivierNicole commented Mar 6, 2025