From 69422f01c27af33618952374af787e18e781719e Mon Sep 17 00:00:00 2001 From: Olivier Nicole Date: Tue, 28 Jan 2025 16:11:46 +0100 Subject: [PATCH 01/15] Run all tests with double translation too --- .github/workflows/build.yml | 5 ++++- compiler/tests-jsoo/lib-effects-2/dune | 9 +++++++++ compiler/tests-ocaml/effects-2/dune | 9 +++++++++ dune | 16 ++++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a9bb1252ec..55918a156c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -148,7 +148,10 @@ jobs: - run: opam exec -- make tests if: ${{ !matrix.skip-test }} - - run: opam exec -- dune build @all @runtest @runtest-js --profile with-effects + - run: | + opam exec -- dune build @all @runtest @runtest-js --profile with-effects + opam exec -- dune clean + opam exec -- dune build @all @runtest @runtest-js --profile with-effects-double-translation if: ${{ !matrix.skip-effects }} - run: opam exec -- git diff --exit-code diff --git a/compiler/tests-jsoo/lib-effects-2/dune b/compiler/tests-jsoo/lib-effects-2/dune index ad5f8e40ab..96dfae6623 100644 --- a/compiler/tests-jsoo/lib-effects-2/dune +++ b/compiler/tests-jsoo/lib-effects-2/dune @@ -1,4 +1,13 @@ (env + (with-effects-double-translation + (flags + (:standard -w -38)) + (js_of_ocaml + ;; separate compilation doesn't yet work when using + ;; '--effect=double-translation' since Dune doesn't know it should compile a + ;; different version of the dependencies. + ;; TODO: remove once support in ocaml/dune#11222 is released. + (compilation_mode whole_program))) (_ (flags (:standard -w -38)) diff --git a/compiler/tests-ocaml/effects-2/dune b/compiler/tests-ocaml/effects-2/dune index 57170ed2e5..7bffc87bd7 100644 --- a/compiler/tests-ocaml/effects-2/dune +++ b/compiler/tests-ocaml/effects-2/dune @@ -8,6 +8,15 @@ ;; different version of the dependencies. ;; TODO: remove once support in ocaml/dune#11222 is released. (compilation_mode whole_program))) + (with-effects-double-translation + (flags + (:standard -w -38)) + (js_of_ocaml + ;; separate compilation doesn't yet work when using + ;; '--effect=double-translation' since Dune doesn't know it should compile a + ;; different version of the dependencies. + ;; TODO: remove once support in ocaml/dune#11222 is released. + (compilation_mode whole_program))) (_ (js_of_ocaml (flags diff --git a/dune b/dune index d5e65a5456..764f7d4121 100644 --- a/dune +++ b/dune @@ -21,6 +21,22 @@ (binaries (tools/node_wrapper.exe as node) (tools/node_wrapper.exe as node.exe))) + (with-effects-double-translation + (js_of_ocaml + (compilation_mode separate) + (flags + (:standard --enable effects --effects double-translation)) + (build_runtime_flags + (:standard --enable effects --effects double-translation))) + (wasm_of_ocaml + (compilation_mode separate) + (flags + (:standard --enable effects)) + (build_runtime_flags + (:standard --enable effects))) + (binaries + (tools/node_wrapper.exe as node) + (tools/node_wrapper.exe as node.exe))) (bench_no_debug (flags (:standard \ -g)) From 91168b3441b09740fd5ad4ac451d0b9eb684e5fa Mon Sep 17 00:00:00 2001 From: Olivier Nicole Date: Wed, 29 Jan 2025 10:44:03 +0100 Subject: [PATCH 02/15] CR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jérôme Vouillon --- compiler/tests-jsoo/lib-effects-2/dune | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler/tests-jsoo/lib-effects-2/dune b/compiler/tests-jsoo/lib-effects-2/dune index 96dfae6623..22cc115264 100644 --- a/compiler/tests-jsoo/lib-effects-2/dune +++ b/compiler/tests-jsoo/lib-effects-2/dune @@ -1,13 +1,7 @@ (env (with-effects-double-translation (flags - (:standard -w -38)) - (js_of_ocaml - ;; separate compilation doesn't yet work when using - ;; '--effect=double-translation' since Dune doesn't know it should compile a - ;; different version of the dependencies. - ;; TODO: remove once support in ocaml/dune#11222 is released. - (compilation_mode whole_program))) + (:standard -w -38))) (_ (flags (:standard -w -38)) From 897bd199cfa40313b04f32d5cba51f90030e5e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Wed, 29 Jan 2025 13:49:14 +0100 Subject: [PATCH 03/15] Tests: adjust lib/test/test_fun_call.ml --- lib/tests/test_fun_call.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tests/test_fun_call.ml b/lib/tests/test_fun_call.ml index 69b914db8b..e236435b38 100644 --- a/lib/tests/test_fun_call.ml +++ b/lib/tests/test_fun_call.ml @@ -28,7 +28,7 @@ let s x = if(x === undefined) return "undefined" if(typeof x === "function") - return "function#" + x.length + "#" + x.l + return "function#" + (x.l > 0 ? x.l : x.length) + "#" + x.l if(x.toString() == "[object Arguments]") return "(Arguments: " + Array.prototype.slice.call(x).toString() + ")"; return x.toString() From 2a5ef522bcababdec576c96a90c3904339011ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Wed, 29 Jan 2025 13:48:54 +0100 Subject: [PATCH 04/15] Run tests with a larger stack --- tools/node_wrapper.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/node_wrapper.ml b/tools/node_wrapper.ml index 6cb569e4bc..281c7d4bfc 100644 --- a/tools/node_wrapper.ml +++ b/tools/node_wrapper.ml @@ -4,6 +4,8 @@ let extra_args_for_wasoo = ; "--stack-size=10000" ] +let extra_args_for_jsoo = [ "--stack-size=3000" ] + let env = Unix.environment () let env = @@ -28,7 +30,7 @@ let args = match argv with | file :: _ when Filename.check_suffix file ".wasm.js" -> extra_args_for_wasoo @ argv - | _ -> argv + | _ -> extra_args_for_jsoo @ argv in Array.of_list (exe :: argv) | [] -> assert false From d92f2681872706f824e8ca4b4b765d07f877e8a3 Mon Sep 17 00:00:00 2001 From: Olivier Nicole Date: Wed, 29 Jan 2025 16:14:37 +0100 Subject: [PATCH 05/15] Fixes --- .github/workflows/build.yml | 1 - compiler/tests-dynlink-js/effects_flags.ml | 2 +- compiler/tests-ocaml/effects-2/dune | 8 +------- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 55918a156c..9b3ff52d01 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -150,7 +150,6 @@ jobs: - run: | opam exec -- dune build @all @runtest @runtest-js --profile with-effects - opam exec -- dune clean opam exec -- dune build @all @runtest @runtest-js --profile with-effects-double-translation if: ${{ !matrix.skip-effects }} diff --git a/compiler/tests-dynlink-js/effects_flags.ml b/compiler/tests-dynlink-js/effects_flags.ml index ef0fe616ba..66db6ee186 100644 --- a/compiler/tests-dynlink-js/effects_flags.ml +++ b/compiler/tests-dynlink-js/effects_flags.ml @@ -6,7 +6,7 @@ let () = let major = String.split_on_char '.' Sys.ocaml_version |> List.hd |> int_of_string in let has_effect l = match l with - | [ "with-effects" ] -> major >= 5 + | [ "with-effects" | "with-effects-double-translation" ] -> major >= 5 | _ -> false in let aux l = enable (has_effect l) "effects" in diff --git a/compiler/tests-ocaml/effects-2/dune b/compiler/tests-ocaml/effects-2/dune index 7bffc87bd7..2cebed0707 100644 --- a/compiler/tests-ocaml/effects-2/dune +++ b/compiler/tests-ocaml/effects-2/dune @@ -10,13 +10,7 @@ (compilation_mode whole_program))) (with-effects-double-translation (flags - (:standard -w -38)) - (js_of_ocaml - ;; separate compilation doesn't yet work when using - ;; '--effect=double-translation' since Dune doesn't know it should compile a - ;; different version of the dependencies. - ;; TODO: remove once support in ocaml/dune#11222 is released. - (compilation_mode whole_program))) + (:standard -w -38))) (_ (js_of_ocaml (flags From 508776294373ba15862b4089bb28087bbdf1ce60 Mon Sep 17 00:00:00 2001 From: Olivier Nicole Date: Wed, 29 Jan 2025 16:19:36 +0100 Subject: [PATCH 06/15] Remove unnecessary directories --- compiler/tests-jsoo/lib-effects-2/dune | 66 ------------------------ compiler/tests-ocaml/effects-2/dune | 69 -------------------------- 2 files changed, 135 deletions(-) delete mode 100644 compiler/tests-jsoo/lib-effects-2/dune delete mode 100644 compiler/tests-ocaml/effects-2/dune diff --git a/compiler/tests-jsoo/lib-effects-2/dune b/compiler/tests-jsoo/lib-effects-2/dune deleted file mode 100644 index 22cc115264..0000000000 --- a/compiler/tests-jsoo/lib-effects-2/dune +++ /dev/null @@ -1,66 +0,0 @@ -(env - (with-effects-double-translation - (flags - (:standard -w -38))) - (_ - (flags - (:standard -w -38)) - (js_of_ocaml - (flags - (:standard --effects=double-translation)) - ;; separate compilation doesn't yet work when using - ;; '--effect=double-translation' since Dune doesn't know it should compile a - ;; different version of the dependencies. - ;; TODO: remove once support in ocaml/dune#11222 is released. - (compilation_mode whole_program)))) - -(copy_files# - (only_sources true) - (files ../lib-effects/*.ml)) - -(copy_files - (only_sources true) - (files ../lib-effects/*.expected)) - -(library - (name jsoo_testsuite_effect2) - (enabled_if - (>= %{ocaml_version} 5)) - (inline_tests - (modes js)) - (modules - (:standard - \ - assume_no_perform - assume_no_perform_unhandled - assume_no_perform_nested_handler - deep_state - effects)) - (preprocess - (pps ppx_expect))) - -(tests - (build_if - (>= %{ocaml_version} 5)) - (names effects) - (modules effects) - (modes js)) - -(tests - (build_if - (>= %{ocaml_version} 5)) - (names - assume_no_perform - assume_no_perform_unhandled - assume_no_perform_nested_handler) - (modules - assume_no_perform - assume_no_perform_unhandled - assume_no_perform_nested_handler) - (libraries js_of_ocaml) - (action - (ignore-outputs - (with-accepted-exit-codes - 0 - (run node %{test})))) - (modes js)) diff --git a/compiler/tests-ocaml/effects-2/dune b/compiler/tests-ocaml/effects-2/dune deleted file mode 100644 index 2cebed0707..0000000000 --- a/compiler/tests-ocaml/effects-2/dune +++ /dev/null @@ -1,69 +0,0 @@ -(env - (with-effects - (js_of_ocaml - (flags - (:standard --effects=double-translation)) - ;; separate compilation doesn't yet work when using - ;; '--effect=double-translation' since Dune doesn't know it should compile a - ;; different version of the dependencies. - ;; TODO: remove once support in ocaml/dune#11222 is released. - (compilation_mode whole_program))) - (with-effects-double-translation - (flags - (:standard -w -38))) - (_ - (js_of_ocaml - (flags - (:standard --effects=double-translation)) - ;; separate compilation doesn't yet work when using - ;; '--effect=double-translation' since Dune doesn't know it should compile a - ;; different version of the dependencies. - ;; TODO: remove once support in ocaml/dune#11222 is released. - (compilation_mode whole_program)))) - -(copy_files ../effects/*.expected) - -(copy_files# - (only_sources true) - (files ../effects/*.ml)) - -(tests - (build_if - (>= %{ocaml_version} 5)) - (names - cmphash - marshal - evenodd - manylive - overflow - partial - reperform - sched - shallow_state_io - shallow_state - test10 - test11 - test1 - test2 - test3 - test4 - test5 - test6 - test_lazy - used_cont) - (modules - (:standard \ unhandled_unlinked)) - (modes js)) - -(tests - (build_if - (>= %{ocaml_version} 5)) - (names unhandled_unlinked) - (modules unhandled_unlinked) - (action - (pipe-outputs - (with-accepted-exit-codes - 2 - (run node %{test})) - (run cat))) - (modes js)) From 3b72f35a0b2c9eb3ff369883f529c69b51325180 Mon Sep 17 00:00:00 2001 From: Olivier Nicole Date: Wed, 29 Jan 2025 16:25:53 +0100 Subject: [PATCH 07/15] Pin Dune to a version that supports the new --effects flag --- .github/workflows/build.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9b3ff52d01..8c48dd82ca 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -116,6 +116,11 @@ jobs: with: ocaml-compiler: ${{ matrix.ocaml-compiler }} + # Pin Dune to a version which supports the new --effects flag + # (https://github.com/ocaml/dune/pull/11222). + - name: Pin Dune + run: opam pin add --no-action dune https://github.com/OlivierNicole/dune.git#jsoo-effects + # Work-around a race between reinstalling mingw-w64-shims # (because of conf-pkg-config optional dep) and installing other # packages that implicitly depend on mingw-w64-shims. From 021a206a1f9557481edb1e334ffe928bd8fdc928 Mon Sep 17 00:00:00 2001 From: Olivier Nicole Date: Wed, 29 Jan 2025 17:43:50 +0100 Subject: [PATCH 08/15] Fixes --- .github/workflows/build.yml | 7 ++++--- compiler/tests-dynlink-js/effects_flags.ml | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8c48dd82ca..02aae3e8d5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -153,9 +153,10 @@ jobs: - run: opam exec -- make tests if: ${{ !matrix.skip-test }} - - run: | - opam exec -- dune build @all @runtest @runtest-js --profile with-effects - opam exec -- dune build @all @runtest @runtest-js --profile with-effects-double-translation + - run: opam exec -- dune build @all @runtest @runtest-js --profile with-effects + if: ${{ !matrix.skip-effects }} + + - run: opam exec -- dune build @all @runtest @runtest-js --profile with-effects-double-translation if: ${{ !matrix.skip-effects }} - run: opam exec -- git diff --exit-code diff --git a/compiler/tests-dynlink-js/effects_flags.ml b/compiler/tests-dynlink-js/effects_flags.ml index 66db6ee186..d6b092f585 100644 --- a/compiler/tests-dynlink-js/effects_flags.ml +++ b/compiler/tests-dynlink-js/effects_flags.ml @@ -6,7 +6,7 @@ let () = let major = String.split_on_char '.' Sys.ocaml_version |> List.hd |> int_of_string in let has_effect l = match l with - | [ "with-effects" | "with-effects-double-translation" ] -> major >= 5 + | [ ("with-effects" | "with-effects-double-translation") ] -> major >= 5 | _ -> false in let aux l = enable (has_effect l) "effects" in From 9048a8476797f46984fcf886721e65c8ea8784c2 Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Mon, 3 Feb 2025 11:20:51 +0100 Subject: [PATCH 09/15] small tuning --- compiler/tests-jsoo/lib-effects/dune | 2 ++ compiler/tests-ocaml/effects/dune | 6 +++--- dune | 15 ++++----------- tools/sync_testsuite.ml | 1 - 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/tests-jsoo/lib-effects/dune b/compiler/tests-jsoo/lib-effects/dune index ba160e259d..d3ca13c41f 100644 --- a/compiler/tests-jsoo/lib-effects/dune +++ b/compiler/tests-jsoo/lib-effects/dune @@ -1,4 +1,6 @@ (env + (with-effects-double-translation) + (with-effects) (_ (js_of_ocaml (flags diff --git a/compiler/tests-ocaml/effects/dune b/compiler/tests-ocaml/effects/dune index 0cfa10633b..019935b596 100644 --- a/compiler/tests-ocaml/effects/dune +++ b/compiler/tests-ocaml/effects/dune @@ -1,10 +1,10 @@ (env + (with-effects-double-translation) + (with-effects) (_ (js_of_ocaml (flags - (:standard --enable=effects)) - (build_runtime_flags - (:standard --enable=effects))))) + (:standard --enable effects))))) (tests (build_if diff --git a/dune b/dune index 764f7d4121..a4064b14a9 100644 --- a/dune +++ b/dune @@ -9,14 +9,10 @@ (js_of_ocaml (compilation_mode separate) (flags - (:standard --enable effects)) - (build_runtime_flags (:standard --enable effects))) (wasm_of_ocaml (compilation_mode separate) (flags - (:standard --enable effects)) - (build_runtime_flags (:standard --enable effects))) (binaries (tools/node_wrapper.exe as node) @@ -25,15 +21,12 @@ (js_of_ocaml (compilation_mode separate) (flags - (:standard --enable effects --effects double-translation)) + (:standard --effects double-translation)) (build_runtime_flags - (:standard --enable effects --effects double-translation))) + (:standard --effects double-translation))) (wasm_of_ocaml - (compilation_mode separate) - (flags - (:standard --enable effects)) - (build_runtime_flags - (:standard --enable effects))) + ;; Double transaction is not supported in wasm + (enabled_if false)) (binaries (tools/node_wrapper.exe as node) (tools/node_wrapper.exe as node.exe))) diff --git a/tools/sync_testsuite.ml b/tools/sync_testsuite.ml index 91faeafdcf..7a35b1d35b 100644 --- a/tools/sync_testsuite.ml +++ b/tools/sync_testsuite.ml @@ -150,7 +150,6 @@ let () = | `Tool | `Typing | `Dynlink -> () | `Ignore -> () | `No -> Printf.eprintf "missing expected %s\n" x) - | `Extra (Dir "effects-2") -> () | `Extra (Ml "testing.ml") -> () | `Extra (Ml "expect.ml") -> () | `Extra (Expected x) -> Printf.eprintf "extra expected %s\n" x From 05dec5ffb1a10085c1cfc5a2f1e07da8a3e743b4 Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Mon, 3 Feb 2025 12:26:46 +0100 Subject: [PATCH 10/15] dynlink and toplevel with double-translation --- compiler/tests-dynlink-js/dune | 15 --------------- compiler/tests-dynlink-js/effects_flags.ml | 19 ++++++++----------- compiler/tests-ocaml/effect-syntax/dune | 6 +++--- toplevel/examples/lwt_toplevel/dune | 13 ++----------- .../examples/lwt_toplevel/effects_flags.ml | 18 +++++++++--------- 5 files changed, 22 insertions(+), 49 deletions(-) diff --git a/compiler/tests-dynlink-js/dune b/compiler/tests-dynlink-js/dune index 23e6869ba2..0e1f614bb4 100644 --- a/compiler/tests-dynlink-js/dune +++ b/compiler/tests-dynlink-js/dune @@ -4,27 +4,12 @@ (libraries js_of_ocaml) (link_flags (:standard -linkall)) - (js_of_ocaml - (flags - (:standard) - (:include effects_flags.sexp)) - (build_runtime_flags - (:standard) - (:include effects_flags.sexp)) - (link_flags (:standard))) (modes js byte)) (executable (name effects_flags) (modules effects_flags)) -(rule - (target effects_flags.sexp) - (action - (with-stdout-to - %{target} - (run ./effects_flags.exe sexp %{profile})))) - (rule (target effects_flags.txt) (action diff --git a/compiler/tests-dynlink-js/effects_flags.ml b/compiler/tests-dynlink-js/effects_flags.ml index d6b092f585..e7bb0ce77d 100644 --- a/compiler/tests-dynlink-js/effects_flags.ml +++ b/compiler/tests-dynlink-js/effects_flags.ml @@ -1,19 +1,16 @@ -let enable b n = - let f = if b then "--enable" else "--disable" in - [ f; n ] - let () = let major = String.split_on_char '.' Sys.ocaml_version |> List.hd |> int_of_string in - let has_effect l = - match l with - | [ ("with-effects" | "with-effects-double-translation") ] -> major >= 5 - | _ -> false + let effects_flags l = + match l, major >= 5 with + | [ "with-effects-double-translation" ], true -> [ "--effects"; "double-translation" ] + | [ "with-effects" ], true -> [ "--enable"; "effects" ] + | _, true -> [ "--disable"; "effects" ] + | _, false -> [ "--disable"; "effects" ] in - let aux l = enable (has_effect l) "effects" in match Sys.argv |> Array.to_list |> List.tl with - | "txt" :: rest -> List.iter print_endline (aux rest) + | "txt" :: rest -> List.iter print_endline (effects_flags rest) | "sexp" :: rest -> print_endline "("; - List.iter print_endline (aux rest); + List.iter print_endline (effects_flags rest); print_endline ")" | _ -> assert false diff --git a/compiler/tests-ocaml/effect-syntax/dune b/compiler/tests-ocaml/effect-syntax/dune index 5d1b84052a..22b9fdf7e4 100644 --- a/compiler/tests-ocaml/effect-syntax/dune +++ b/compiler/tests-ocaml/effect-syntax/dune @@ -1,10 +1,10 @@ (env + (with-effects-double-translation) + (with-effects) (_ (js_of_ocaml (flags - (:standard --enable=effects)) - (build_runtime_flags - (:standard --enable=effects))))) + (:standard --enable effects))))) (tests (names diff --git a/toplevel/examples/lwt_toplevel/dune b/toplevel/examples/lwt_toplevel/dune index 65a80eba6e..b7a8b48fe2 100644 --- a/toplevel/examples/lwt_toplevel/dune +++ b/toplevel/examples/lwt_toplevel/dune @@ -55,9 +55,7 @@ --file (:include html_types_path.txt))) (flags - (:standard - --toplevel - (:include effects_flags.sexp)))) + (:standard --toplevel))) (modules (:standard \ test_dynlink examples effects_flags)) (preprocess @@ -140,19 +138,12 @@ (name effects_flags) (modules effects_flags)) -(rule - (target effects_flags.sexp) - (action - (with-stdout-to - %{target} - (run ./effects_flags.exe sexp)))) - (rule (target effects_flags.txt) (action (with-stdout-to %{target} - (run ./effects_flags.exe txt)))) + (run ./effects_flags.exe txt %{profile})))) (rule (targets toplevel.js) diff --git a/toplevel/examples/lwt_toplevel/effects_flags.ml b/toplevel/examples/lwt_toplevel/effects_flags.ml index 4bbf57d282..703c1b21a3 100644 --- a/toplevel/examples/lwt_toplevel/effects_flags.ml +++ b/toplevel/examples/lwt_toplevel/effects_flags.ml @@ -1,15 +1,15 @@ -let enable b n = - let f = if b then "--enable" else "--disable" in - [ f; n ] - let () = let major = String.split_on_char '.' Sys.ocaml_version |> List.hd |> int_of_string in - let has_effect = major >= 5 in - let l = enable has_effect "effects" in + let effects_flags l = + match l, major >= 5 with + | [ "with-effects-double-translation" ], true -> [ "--effects"; "double-translation" ] + | _, true -> [ "--enable"; "effects" ] + | _, false -> [ "--disable"; "effects" ] + in match Sys.argv |> Array.to_list |> List.tl with - | "txt" :: [] -> List.iter print_endline l - | "sexp" :: [] -> + | "txt" :: rest -> List.iter print_endline (effects_flags rest) + | "sexp" :: rest -> print_endline "("; - List.iter print_endline l; + List.iter print_endline (effects_flags rest); print_endline ")" | _ -> assert false From 5d1c5ebaf15766200427f2ad29b009a8940ad37f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Mon, 3 Feb 2025 12:58:34 +0100 Subject: [PATCH 11/15] Add a comment for the larger state size requirement --- tools/node_wrapper.ml | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/node_wrapper.ml b/tools/node_wrapper.ml index 281c7d4bfc..0e28e47477 100644 --- a/tools/node_wrapper.ml +++ b/tools/node_wrapper.ml @@ -5,6 +5,7 @@ let extra_args_for_wasoo = ] let extra_args_for_jsoo = [ "--stack-size=3000" ] +(* We somehow need a larger stack with --effects=double-translation. *) let env = Unix.environment () From c84bb214622533bcaa1172729c80d04d47e8e9df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Mon, 3 Feb 2025 13:21:48 +0100 Subject: [PATCH 12/15] Update effect documentation --- manual/effects.wiki | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/manual/effects.wiki b/manual/effects.wiki index cffe51c226..5794e8bf43 100644 --- a/manual/effects.wiki +++ b/manual/effects.wiki @@ -11,14 +11,13 @@ The analysis is especially effective on monomorphic code. It is not so effective We hope to improve on this by trying alternative compilation strategies. -An alternative CPS transform is provided under the {{--effects=double-translation}} option. It keeps a direct-style version of the transformed functions in addition to the CPS version. The choice of running the CPS version is delayed to run time. Since CPS code is usually slower, this can avoid degradations. In addition, one can ensure that some code is run in direct style by using {{Jsoo_runtime.Effect.assume_no_perform}}. A caveat is that Dune does not know about {{--effects=double-translation}} yet and may try to link together files built with {{--enable=double-translation}} and files built with only {{--enable=effects}}, which gives an error. A work-around is to disable separate compilation by using the option {{(js_of_ocaml (compilation_mode whole_program))}}. +An alternative CPS transform is provided under the {{{--effects=double-translation}}} option. It keeps a direct-style version of the transformed functions in addition to the CPS version. The choice of running the CPS version is delayed to run time. Since CPS code is usually slower, this can avoid degradations. In addition, one can ensure that some code is run in direct style by using {{{Jsoo_runtime.Effect.assume_no_perform}}}. === Dune integration === -We're still working on dune support for compiling js_of_ocaml programs -with effect handlers enabled. +Dune is aware of the {{{--enable effects}}} option. So, you can just add it to the Js_of_ocaml flags {{{(js_of_ocaml (flags ...))}}} wherever you want to use it. -For now, here are two possible setups. +We're still working on dune support for the {{{--effects}}} option. For now, here are two possible setups. === Whole dune workspace setup === @@ -27,8 +26,8 @@ Put the following in a {{{dune}}} (or {{{dune-workspace}}}) file at the root of (env (_ (js_of_ocaml - (flags (:standard --enable effects)) - (build_runtime_flags (:standard --enable effects))))) + (flags (:standard --effects=double-translation)) + (build_runtime_flags (:standard --effects=double-translation))))) }}} With this setup, one can use both separate and whole program compilation. @@ -37,28 +36,19 @@ With this setup, one can use both separate and whole program compilation. === Sub directory setup === If you want to enable effect handlers for some binaries only, you'll -have to give-up separate compilation for now using the following in -your {{{dune}}} file. - -{{{ -(env - (_ - (js_of_ocaml - (compilation_mode whole_program)))) -}}} - -Then pass the rights {{{js_of_ocaml}}} flags to the executable stanza - +have to give-up separate compilation for now using: {{{ (executable (name main) - (js_of_ocaml (flags (:standard --enable effects))) + (js_of_ocaml + (compilation_mode whole_program) + (flags (:standard --effects=double-translation))) ) }}} Trying to use separate compilation would result in a error while attempting to link the final js file. {{{ js_of_ocaml: Error: Incompatible build info detected while linking. - - test6.bc.runtime.js: effects=false - - .cmphash.eobjs/byte/dune__exe.cmo.js: effects=true + - test6.bc.runtime.js: effects=disabled + - .cmphash.eobjs/byte/dune__exe.cmo.js: effects=double-translation }}} From abdb9b56d3870079de9852eb6be8a0f4603ce78e Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Mon, 3 Feb 2025 13:48:54 +0100 Subject: [PATCH 13/15] don't set stack-size if not needed --- tools/dune | 13 ++++++++++++- tools/node_wrapper.ml | 3 +-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/dune b/tools/dune index 5953c6bc08..721a9b48b9 100644 --- a/tools/dune +++ b/tools/dune @@ -1,8 +1,19 @@ (executable (name node_wrapper) - (modules node_wrapper) + (modules node_wrapper node_wrapper_per_profile) (libraries unix)) +(executable + (name gen_node_wrapper_per_profile) + (modules gen_node_wrapper_per_profile)) + +(rule + (target node_wrapper_per_profile.ml) + (action + (with-stdout-to + %{target} + (run ./gen_node_wrapper_per_profile.exe %{profile})))) + (executable (name ci_setup) (modules ci_setup) diff --git a/tools/node_wrapper.ml b/tools/node_wrapper.ml index 0e28e47477..15979975a8 100644 --- a/tools/node_wrapper.ml +++ b/tools/node_wrapper.ml @@ -4,8 +4,7 @@ let extra_args_for_wasoo = ; "--stack-size=10000" ] -let extra_args_for_jsoo = [ "--stack-size=3000" ] -(* We somehow need a larger stack with --effects=double-translation. *) +let extra_args_for_jsoo = [] @ Node_wrapper_per_profile.args let env = Unix.environment () From 3b05f82a53842f9eea8f7a362757b3295afb5ca2 Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Mon, 3 Feb 2025 13:49:06 +0100 Subject: [PATCH 14/15] don't set stack-size if not needed --- tools/gen_node_wrapper_per_profile.ml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tools/gen_node_wrapper_per_profile.ml diff --git a/tools/gen_node_wrapper_per_profile.ml b/tools/gen_node_wrapper_per_profile.ml new file mode 100644 index 0000000000..879f9c5825 --- /dev/null +++ b/tools/gen_node_wrapper_per_profile.ml @@ -0,0 +1,12 @@ +let profile = Sys.argv.(1) + +let () = + match profile with + | "with-effects-double-translation" -> + (* We somehow need a larger stack with --effects=double-translation. *) + print_endline {| +let args = [ "--stack-size=3000" ] +|} + | _ -> print_endline {| +let args = [ ] +|} From d44c7ba4209ef46469960beb22b84c2b5eb1fea9 Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Mon, 3 Feb 2025 14:30:01 +0100 Subject: [PATCH 15/15] fix --- tools/ci_setup.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/ci_setup.ml b/tools/ci_setup.ml index f361e6fecd..3156fbd0c3 100644 --- a/tools/ci_setup.ml +++ b/tools/ci_setup.ml @@ -64,7 +64,8 @@ let node_wrapper = , {|(executable (public_name node) (name node_wrapper) - (libraries unix))|} ) + (libraries unix))|} ) + ; "node_wrapper/node_wrapper_per_profile.ml", {|let args = []|} ; "node_wrapper/dune-project", "(lang dune 3.17)" ; "node_wrapper/node_wrapper.opam", "" ]