-
Notifications
You must be signed in to change notification settings - Fork 606
V8: Enable as unstable, add syscalls, flesh out call_reducer, etc. #3276
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
base: master
Are you sure you want to change the base?
Conversation
Is this meant to be reviewed or just here for a demo? |
It's meant to be reviewed. |
5e12f1a
to
e5f893d
Compare
.requires("wasm_file") | ||
.hide(true) | ||
.help("UNSTABLE: interpret `--bin-path` as a JS module"), | ||
) |
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 haven't been keeping context on the timelines/milestones for the V8 stuff. Is the "demo" here meaning an internal demo, or a beta release to users?
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.
More of the former. I'm not sure this is the UX we want to expose to beta users, but I'd like to merge something provisional into master that lets us test stuff. I'd love to discuss with you what a good UX would be, but we can probably do that on Discord for now.
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 discussed this with @Centril today in the Core daily meeting. For the beta release we decided to have parity with the UX of both Rust and C#, so this will not be necessary because SpacetimeDB will detect that you have an npm package and will build your app for you.
// i.e. This will just work
spacetime publish
However, we also want to achieve parity with manual way of specifying the --bin-path
arg for wasm by introducing a new --js-path
manual way of publishing after having already bundled your javascript.
Once @Centril makes that change I will approve this PR for the files that I am a code owner of.
# Description of Changes Implements `Serialize` and `Deserialize` for tuples. Extracted from #3276. # API and ABI breaking changes None # Expected complexity level and risk 2 # Testing A test `roundtrip_tuples_in_different_data_formats` is added.
69222d0
to
e563b93
Compare
0454ff8
to
cfc090d
Compare
Description of Changes
This PR:
unstable
feature flag on the host. To publish a JS module,--javascript --bin-path path/to/module.js
needs to be used. This mechanism is provisional until we come up with something better.WasmInstanceEnv
toInstanceEnv
and friends.create_instance
,make_actor
,call_reducer
with timeouts and long-running logs added as well.API and ABI breaking changes
None
Expected complexity level and risk
3? It's only available on unstable and mostly touches V8 stuff, but the PR is fairly big.
Testing
Follow up PRs will add unit tests for parts.
We'll also need to add integration tests for whole modules.