update jsoo for testing

[phated]

opening this draft for windows CI testing purposes

[hhugo]

Try with this diff

diff --git a/compiler/test/dune b/compiler/test/dune
index a8b92d0c..111938bc 100644
--- a/compiler/test/dune
+++ b/compiler/test/dune
@@ -22,4 +22,4 @@
  (libraries grain grain-tests.framework grain-tests.suites grain-tests.utils)
  (modules test)
  (js_of_ocaml
-  (flags --no-sourcemap --quiet)))
+  (flags --no-sourcemap --quiet --disable use-js-string)))

[hhugo]

I'm wondering if the slowdown could be due to yojson reading "cmi" pattern

[hhugo]

The current implementation:

• first read the cmi section into some bytes
• convert bytes to string
• Yojson.from_string convert the string back into bytes

An alternative approach can mitigate the perf issue
hhugo@543d5a9

[phated]

Thanks @hhugo! I've cherry-picked that commit into this branch to see it in action. Any idea why this issue is only showing up now (with this upgrade)?

[hhugo]

Thanks @hhugo! I've cherry-picked that commit into this branch to see it in action. Any idea why this issue is only showing up now (with this upgrade)?

js_of_ocaml-compiler.5.1.0 changed the default representation of strings.

[hhugo]

With the commit, you no longer need to disable use-js-string.
I've created a branch with additional changes.
see https://github.com/hhugo/grain/tree/hhugo-jsoo-oom

[phated]

@hhugo the tests have still gone from 3-4 minutes to 16 minutes with the newer JSOO. This seems untenable for us since our compiler binaries are produced via jsoo. Do you have any other ideas why there would be a 400% slowdown between the versions?

[hhugo]

@phated, would you be able to rebase this branch, I have some time to look at this again.

[ospencer]

@hhugo I rebased this. We changed the way a lot of the code that we previously thought needed updating works, so once the CI finishes we should see how long the tests take and then go from there. Thanks for looking into this again!

[hhugo]

@phated, I've identified a change that make every thing slow.
ocsigen/js_of_ocaml#1409

You enable backtraces unconditionally in module_resolution.
Jsoo will create a js Error for every exception raised to capture backtraces.

Try enabling backtraces only when not running with jsoo.

let () = Printexc.record_backtrace(Sys.backend_type != (Other("js_of_ocaml")));

[hhugo]

I've make some cleanup in https://github.com/hhugo/grain/tree/cleanup but it's missing an upgrade to js_of_ocaml-compiler.5.8.2.
I don't know how to use esy properly.

[ospencer]

@hhugo thanks! Locally, at least, that seems to be the issue. I committed that change here so we can see how fast the tests run. We'll likely also pull the rest of your cleanup into this branch. Really appreciate you looking into this!

[ospencer]

It looks like the tests are still running slow. Locally I just compiled a program and compared the performance against main and with the changes it seemed to be roughly the same, with the new jsoo being a tad slower than before. I ran the tests locally and confirmed that they're running slow there too. I also disabled the backtraces for the tests. Looks like they're 3-4x slower than before.

[hhugo]

I think the issue is that 3rd party libs enable backtraces. See https://github.com/reasonml/reason-native/blob/20b1997b6451d9715dfdbeec86a9d274c7430ed8/src/rely/Util.re#L12

You can try overriding caml_record_backtrace with

//Provides: caml_record_backtrace
   function caml_record_backtrace(b){
    // caml_record_backtrace_flag = b;
    return 0;
   }

[hhugo]

Here is what I have locally after tweaking the generated code manually.

$ node _esy/default/store/b/grain__s__compiler-dc7bdbee/default/test/test.bc.js 

Running 45 test suites
 PASS  aliased types
 PASS  abstract types
 PASS  recursive types
 PASS  function types
 PASS  tuples
 PASS  strings
 PASS  stdlib
 PASS  early return
 PASS  records
 PASS  provides
 PASS  pattern matching
 PASS  parsing
 PASS  optimizations
 PASS  numbers
 PASS  modules
 PASS  loops
 PASS  lists
 PASS  linking
 PASS  let mut
 PASS  includes
 PASS  functions
 PASS  foreigns
 PASS  exceptions
 PASS  enums
 PASS  cyclic redundancy checks
 PASS  comments
 PASS  chars
 PASS  boxes
 PASS  blocks
 PASS  basic functionality
 PASS  arrays
 PASS  aux/wasm_utils
 PASS  utils/string_utils
 PASS  utils/mini_bigint
 PASS  utils/markdown
 PASS  utils/literals
 PASS  aux/concatlist

Test Suites: 0 failed, 8 skipped, 37 passed, 45 total
Tests:       0 failed, 405 skipped, 784 passed, 1189 total
Time:        131.838s

[ospencer]

Gotcha, that looks like it. Just pushed that change.

[hhugo]

@ospencer, I've just merged ocsigen/js_of_ocaml#1637, would you be able to test js_of_ocaml-compiler master here ? (dropping the compiler/test/hacks.js override)

[ospencer]

@hhugo Apologies for the delay, I just pushed that change.