-
Notifications
You must be signed in to change notification settings - Fork 9
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
Separate compilation #36
Conversation
We were just numbering them. This should make the output more readable since we are propagating names from the OCaml source code.
Directly use the name from the OCaml code if possible, with a suffix in case of ambiguity. Use short names for not explictly named variables.
96621d6
to
136bd6d
Compare
function fetchRelative(src) { | ||
const base = globalThis?.document?.currentScript?.src; | ||
const url = base?new URL(src, base):src; | ||
return fetch(url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The number of modules definitely seems to be an issue. I had to change the impl of fetchRelative
to something incredibly hacky[1] to be able to load my app this way. I think we need to either explore the cmo idea more seriously or come up with something smarter here.
[1] For future reference, I made fetchRelative
take i
and return new Promise(resolve => setTimeout(() => resolve(url), i)).then(fetch)
where i
is the index of module
in link
. If I do this, then I can run apps with many more times the modules than would be possible without it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe trying to load as many modules as possible in parallel was not such a good idea... I should limit the amount of parallelism. Maybe you could try swapping these two lines?
const code = loadCode(src + "/" + module[0] + ".wasm")
await Promise.all(sync?deps:module[1].map((i)=>deps[i]));
Anyway, having so many modules is slow, so I'm planning to move back to generating a single Wasm module per cma file.
let dir = Filename.chop_extension output_file ^ ".assets" in | ||
Fs.gen_file dir | ||
@@ fun tmp_dir -> | ||
Sys.mkdir tmp_dir 0o777; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting pretty consistent errors here in dune
with the rules I've written on rebuilds around the dir already existing. Is this working for you (with rebuilds) on the version of dune in ocaml/dune#8278?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should try https://github.com/ocaml-wasm/dune/tree/wasm-separate-compilation. The code is not quite correct (I cherry-picked ocaml/dune#10331 which is just incorrect), but I think it works well enough for now.
6ea1bbe
to
3879f7a
Compare
Improve Wat output
3879f7a
to
92080d2
Compare
The compilation process is basically the same as with Js_of_ocaml. One needs to build a standalone runtime:
Then, one compiles .cma and .cmo files.
Finally, one links all these files.
Besides a JavaScript file
program.js
, this generates aprogram.assets
directory containing Wasm binary files and source map files.At the moment, one Wasm module is produced for each OCaml module, but the V8 Wasm engine is somewhat struggling with so many modules, so I should probably reconsider this.
Another improvement would be to be able to compile each cmo files in a library separately before linking them together, as suggested by @hhugo in ocaml/dune#7389.