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

[fl_dynload] Allow overriding low-level module loader #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ejgallego
Copy link

Before this patch, Fl_dynload.load_packages will unconditionally call Dynlink.loadfile to load each particular compilation unit.

This works fine in almost all our scenarios, however when compiling Web Workers with JSOO, we have found that Dynlink.loadfile takes a very long time as it will compile .cma files to .js.

In the case of jsCoq and coq-lsp this compilation process is too heavy. Prior to us using findlib, we used to trap all calls from Coq to Dynlink.loadfile, and we used a pre-compiled .js version which was much much faster.

This patch makes it possible for users of Fl_dynload.load_packages to override the call to Dynlink.loadfile as to for example implement the above cache setup, or to do other kinds of instrumentations.

Before this patch, `Fl_dynload.load_packages` will unconditionally
call `Dynlink.loadfile` to load each particular compilation unit.

This works fine in almost all our scenarios, however when compiling
Web Workers with JSOO, we have found that `Dynlink.loadfile` takes a
very long time as it will compile `.cma` files to `.js`.

In the case of jsCoq and `coq-lsp` this compilation process is too
heavy. Prior to us using `findlib`, we used to trap all calls from Coq
to `Dynlink.loadfile`, and we used a pre-compiled `.js` version which
was much much faster.

This patch makes it possible for users of `Fl_dynload.load_packages`
to override the call to `Dynlink.loadfile` as to for example implement
the above cache setup, or to do other kinds of instrumentations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant