ocsigen / js_of_ocaml

Compiler from OCaml to Javascript.
http://ocsigen.org/js_of_ocaml/
Other
960 stars 188 forks source link

[BUG] Don't rely on browsers to implement tail-call optimizations / have large stacks #1530

Closed JasonGross closed 12 months ago

JasonGross commented 12 months ago

Describe the bug This js_of_ocaml output fails on Chrome and Firefox with a "too much recursion" or "maximum call stack size exceeded", but (I believe) works fine on Safari which implements tail-call optimization (or provides a larger stack).

Here's a self-contained version of the webpage and the OCaml source file: fiat-html.tar.gz Working on autodeployment at https://github.com/mit-plv/fiat-crypto/pull/1737, so you can see the build on CI there. Warning: it takes about 30 seconds to compile the bytecode (-package js_of_ocaml -package unix -w -20 -g -linkpkg), and about 10 minutes to run js_of_ocaml (with --source-map --no-inline; about twice that without any arguments).

Expected behavior It should be possible to instruct js_of_ocaml to produce javascript that uses less stack.

Versions Version of packages used to reproduce the bug

$ ocamlc -v
The OCaml compiler, version 4.14.1
Standard library directory: /home/jgross/.opam/4.14.1+fp/lib/ocaml
$ js_of_ocaml --version
5.4.0
hhugo commented 12 months ago

To give a bit more context to other readers, the code mentioned here consist of a generated file (using coq) of over a million lines (74Mb).

The following actions should be able to fix your issue:

JasonGross commented 12 months ago

Ah, neat, somehow I missed --enable=effects, this is exactly what I was looking for, thanks! (The naming seems a bit strange though.)

Is --disable compact documented?

hhugo commented 12 months ago

Ah, neat, somehow I missed --enable=effects, this is exactly what I was looking for, thanks! (The naming seems a bit strange though.)

The naming is a bit strange because the it is supposed to be used when using ocaml 5 effects only. We should probably advertise it in other cases and rename it.

Is --disable compact documented?

I don't think and I suspect end-user should not use it. I'll file a bug-report to track the quadratic behavior.

JasonGross commented 12 months ago

Is this also related to having a heavy memory footprint? I'm seeing ~4.4GB RAM (real: 190.10, user: 185.48, sys: 3.84, mem: 4476016 kb) even with --disable compact. (Also is --disable compact supposed to significantly increase .js file size? I'm seeing 13MB with --disable compact or 9.3MB without, which I guess is somewhat bigger but is not really much compared to the 79MB .map file or the 74MB source file.)

OlivierNicole commented 12 months ago

Ah, neat, somehow I missed --enable=effects, this is exactly what I was looking for, thanks! (The naming seems a bit strange though.)

For the record, it fixes stack overflows because it installs a trampoline, which is a mechanism that clears the call stack every time it grows beyond a fixed threshold.

hhugo commented 8 months ago

I've pushed a change on master. You hopefully won't need --disable compact anymore (with the master branch).