-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[sdk] Add wasm interpreter support. #5924
Conversation
sdks/builds/setup-emsdk.sh
Outdated
@@ -0,0 +1,14 @@ | |||
|
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.
why use a separate sh file?
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 script was a lot more convoluted before. I'll merge.
sdks/builds/wasm.mk
Outdated
|
||
#toolchain code | ||
.stamp-wasm-toolchain: | ||
@./setup-emsdk.sh $(TOP)/sdks/builds/toolchains/emsdk |
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.
please do not use @
, we don't use it anywhere else
--host=i386-apple-darwin10 | ||
|
||
#toolchain code | ||
.stamp-wasm-toolchain: |
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.
Why do we have a wasm
while all other targets are wasm-interp
?
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.
cuz the other wasm targets require their own toolchains.
.PHONY: clean-wasm-interp clean-wasm | ||
clean-wasm:: | ||
rm -rf .stamp-wasm-toolchain | ||
clean-wasm-interp:: clean-wasm |
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.
please unify both wasm-interp
and wasm
targets as there is no apparent reason to have wasm
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.
There's a reason for this.
There are 4 more runtime configs coming down the line: mini aot/runtime, llvm aot/runtime.
This is the only one ready for merging and I feel like keeping the bits for the others made sense.
@luhenry I updated this PR with the rest of the required work to bring the interpreter in. |
Here's a sample run from the wasm dir: https://gist.github.com/11180dc8613200c0d22a0ba4571ba674 |
sdks/wasm/test.js
Outdated
Module.setValue (args_mem + i * 4, args [i], "i32"); | ||
Module.setValue (eh_throw, 0, "i32"); | ||
|
||
var res = invoke_method (send_message, this_arg, args_mem, eh_throw); |
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.
send_message
should be method
(it's only working as send_message
because that's the only method that gets used in this test file)
sdks/wasm/test.js
Outdated
if (Module.getValue (eh_throw, "i32") != 0) { | ||
Module.Runtime.stackRestore(stack); | ||
var msg = conv_string (res); | ||
throw new SharpException (msg); //the convention is that invoke_method ToString () any outgoing exception |
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.
It's much better if this is just throw new Error(msg)
instead of using SharpException
, because SharpException
causes the error not to appear correctly in the browser debug console.
There might be some way of having a custom Error
subclass that appears properly in the console, but I'm not certain what it is.
sdks/wasm/library_mono.js
Outdated
|
||
request_gc_cycle: function () { | ||
++MONO.pump_count; | ||
if (Module.ENVIRONMENT_IS_WEB) { |
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 don't think this is correct. In mono.js
, ENVIRONMENT_IS_WEB
is a local variable, and is not exposed as a property of Module
, so this check always evaluates as false and GC never runs. I think you need something like the following instead:
if (Module['ENVIRONMENT'] === 'WEB' || Module['ENVIRONMENT'] === 'WORKER')
... which not only fixes the WEB
case but also allows GC in the WORKER
case.
Addressed @SteveSandersonMS issues. |
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.
amazing stuff 👍
So, is xamarin.wasm getting close? |
@juepiezhongren yes, you can try it today if you adventure yourself on building it from source. |
Wow excellent work here @kumpera. Thank you for your time and your contributions! |
@Mike-EEE have u seen the perspective future that x.forms is to fulfill your ubUI? |
Not yet, @juepiezhongren ... not officially at least. From what I understand, the future is somewhere between Xamarin.Forms, Noesis, and Avalonia. Once mono supports (or rather releases support for :)) WebAssembly, each one of these models will be able to run everywhere. I myself am partial to Noesis/Avalonia models, but there is a market for a XF-type model, too. So everyone is (or will be) covered. 😄 |
Ava is still at baby stage, what is Noesis, please give a url, por favor@Mike-EEE |
They are a commercial solution but hold true to WPF. :) |
@Mike-EEE What i prefer most is the architecture for xf, with native binding like x.ios, x.android.... and with Ub solution. So, i hope competitor like ava and noesis could reuse this architecture, to take advantage of native bindings. Ub is always not covering everything, things like react native is only to make things more and more complicated and is never to be rootly solved. |
@Mike-EEE noesis's web page is cool |
Like I said @juepiezhongren everyone should be covered, one way or another. You can tell this space is gaining momentum again and there are a lot of creative minds that are starting to explore and contribute. Far cry from a year or so ago, huh! 😆 |
[sdk] Add wasm interpreter support. Commit migrated from mono/mono@a62a7e7
This adds all bits to get the mini test suite to run under the interpreter