Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Pre-compiled Dynlink for .cma files without linking compiler-libs/js_of_ocaml-compiler #1675

Closed
ejgallego opened this issue Sep 17, 2024 · 5 comments

Comments

@ejgallego
Copy link
Contributor

ejgallego commented Sep 17, 2024

Is your feature request related to a problem? Please describe.

jsCoq usually requires to Dynlink.loadfile of some heavy .cma files (plugins in Coq terminology to work.

Loading such files with JSOO Dynlink.loadfile support works fine, but it is usually quite slow, and moreover increases the output .js file by almost 3 MiB (50%) as it will link js_of_ocaml-compiler.

Describe the solution you'd like

I'd be great if we could precompile the .cma files to .js files, and load them without having to link the compiler.

Describe alternatives you've considered

For jsCoq and JSOO 4.0.0 , this used to work. Updating to JSOO 5.8.3 is done here https://github.com/jscoq/jscoq/pull/372/files#diff-89191e3161815dcdaf5594e6d98bfaa5ab81f742ccbeb9f5a657655c27504cdcR16, that also works, however it increases the binary size by almost 50% as we need to link the compiler.

I tried to provide my own version of toplevelReloc , which seems to be the only missing primitive for this to work, but without success.

@hhugo
Copy link
Member

hhugo commented Sep 17, 2024

There is a tests checking that one can load precompiled cmo successfully. It lives in compiler/tests-dynlink-js.

It seems that there is an issue if one compile the main program with the jsoo --dynlink flags but without linking js_of_ocaml-compiler.dynlink.

Do you really need --dynlink ?

Alternatively, the following patch/hack should fix your issue.

 //Provides: caml_register_global (const, shallow, const)
 //Requires: caml_global_data, caml_callback, caml_build_symbols
 //Requires: caml_failwith
 function caml_register_global(n, v, name_opt) {
   if (name_opt) {
     var name = name_opt;
     if (globalThis.toplevelReloc) {
       n = caml_callback(globalThis.toplevelReloc, [name]);
     } else if (caml_global_data.symbols) {
       if (!caml_global_data.symidx) {
         caml_global_data.symidx = caml_build_symbols(caml_global_data.symbols);
       }
       var nid = caml_global_data.symidx[name];
       if (nid >= 0) n = nid;
       else {
-        caml_failwith("caml_register_global: cannot locate " + name);
+        n = caml_global_data.length - 1;
       }
     }

@ejgallego
Copy link
Contributor Author

Thanks @hhugo , I confirm that with #1676 this problem is solved.

I don't fully understand all the details, but both using --linkall and --dynlink work now. I guess the first option is the preferred one, right?

@hhugo
Copy link
Member

hhugo commented Sep 19, 2024

The thing is that lhe ocamlc link flag -linkall is supposed to be propagated by dune to js_of_ocaml (but it is sometime not).
Ideally, you don't need to pass linkall to jsoo explicitly .

@hhugo
Copy link
Member

hhugo commented Sep 19, 2024

fixed by #1676

@hhugo hhugo closed this as completed Sep 19, 2024
@ejgallego
Copy link
Contributor Author

Thanks a lot @hhugo !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants