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

[Wasm] Add support for P/Invoke through dynamic linking #14259

Merged
merged 23 commits into from May 5, 2019

Conversation

jeromelaban
Copy link
Contributor

@jeromelaban jeromelaban commented Apr 29, 2019

@jeromelaban jeromelaban marked this pull request as ready for review April 29, 2019 17:45
mono/utils/mono-dl-wasm.c Outdated Show resolved Hide resolved
@jeromelaban
Copy link
Contributor Author

jeromelaban commented Apr 30, 2019

So this PR can't be merged as-is because of the dlopen call that I included, which makes emscripten call abort when MAIN_MODULE is not defined.

To be able to have an optional runtime, and avoid having to do -s EXPORT_ALL=1 for the release runtime, there are two options that I can see:

  • either adding a function built here in all three configurations
  • or enable -s MAIN_MODULE=1 in all configurations, with only -s EXPORT_ALL=1 in release-dynamic

I'll make some tests to determine the size of mono.wasm with only -s MAIN_MODULE=1 as it does not seem to include a lot of js code.

@vargaz
Copy link
Contributor

vargaz commented Apr 30, 2019

We already have a pinvoke callback in driver.c called wasm_dl_load ()/wasm_dl_symbol () maybe the dlopen() etc. can be added there.

@jeromelaban
Copy link
Contributor Author

Well, adding MAIN_MODULE=1 makes mono.wasm release go from 1.8MB to 2.6MB, so that's not an option. I'll look for altering the driver, and most probably build it for each configuration to alter dlopen's behavior.

@jeromelaban
Copy link
Contributor Author

@vargaz would something like this be acceptable ?

mono_dl_fallback_register (gboolean allow_dlopen, MonoDlFallbackLoad load_func, MonoDlFallbackSymbol symbol_func, MonoDlFallbackClose close_func, void *user_data)

so dlopen is controled from there, or another method that would be in mono-dl-wasm.c like mono_dl_disable_dlopen() ?

@vargaz
Copy link
Contributor

vargaz commented Apr 30, 2019

Can't wasm_dl_load () in driver.c call dlopen () if some define is set ?

@jeromelaban
Copy link
Contributor Author

Ah I see. You'd redirect dlopen from there, I'll try that, thanks!

@jeromelaban
Copy link
Contributor Author

I need to make another change to avoid dlopen ordering issues.

@jeromelaban
Copy link
Contributor Author

The Linux WebAssembly tests did not pass because they ran out of time (20 minutes). They pass locally on my machine in a container built using the same parameters the CI uses, in 13 minutes. Could it be that the build machine running it is too slow now that the number of tests has grown and that the timeouts should be larger ?

@vargaz
Copy link
Contributor

vargaz commented May 2, 2019

@monojenkins build failed

1 similar comment
@lewing
Copy link
Member

lewing commented May 3, 2019

@monojenkins build failed

@jeromelaban
Copy link
Contributor Author

Good, the wasm test suite passed, the android one is an app crash on android with debugger test suite it seems.

@lewing
Copy link
Member

lewing commented May 3, 2019

@monojenkins build OS X x64 Android SDK

@lewing
Copy link
Member

lewing commented May 3, 2019

@monojenkins build failed

@lewing
Copy link
Member

lewing commented May 3, 2019

@jeromelaban unfortunately this was stuck while I landed the m2n changes. Can you rebase and regenerate?

@jeromelaban
Copy link
Contributor Author

Thanks for the notice. FWIW, your m2n changes may have a positive performance impact for dllimport calls. Trampolines are called very often, particularly in sqlite and skia.

@lewing
Copy link
Member

lewing commented May 4, 2019

Thanks for the update! Yeah, it turns out the m2n stuff had measurable impact in several cases.

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thank you.

@lewing
Copy link
Member

lewing commented May 4, 2019

@monojenkins build Linux AArch64 Interpreter

@lewing
Copy link
Member

lewing commented May 4, 2019

@monojenkins build Linux x64 Interpreter

@lewing
Copy link
Member

lewing commented May 4, 2019

@monojenkins build OS X x64 iOS SDK

@lewing lewing merged commit 8d80ccc into mono:master May 5, 2019
@jeromelaban jeromelaban deleted the dev/jela/wasm-dlopen branch May 6, 2019 15:34
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.

None yet

3 participants