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

Add WebAssembly.validate #752

Merged
merged 1 commit into from Aug 29, 2016
Merged

Add WebAssembly.validate #752

merged 1 commit into from Aug 29, 2016

Conversation

lukewagner
Copy link
Member

This PR adds WebAssembly.validate to the JS API in JS.md. The purpose of validate is to provide a flexible way to feature test by asking whether a module containing the opcode, type, section etc in question validates. WebAssembly.compile or the Module constructor could also be used for this purpose but:

  1. they're going to be a lot slower since they're attempting to compile a module
  2. if we end up with deferred error reporting (Function bodies deferred error reporting #719) one would also have to execute the code in question which adds complexity/cost

This PR proposes:

  • that validate is synchronous, allowing super-fast queries for what I expect will be mostly tiny modules
  • that validate return a simple Boolean; fancier "why didn't it succeed" result messages with strings or error codes sound like portability hazards (if they aren't precisely specified) and add unnecessary cost assuming most uses will simply want the boolean result.

@ckknight
Copy link
Contributor

ckknight commented Aug 6, 2016

I think having the return value be synchronous could be a bad idea, such as if one needs to validate a very large module.

@sunfishcode sunfishcode modified the milestone: MVP Aug 6, 2016
@lukewagner
Copy link
Member Author

It could make sense to add an async validation function (similar to how we have both sync and async ways to compile a module) if we found a compelling use case but, given the use case of feature testing, the modules are expected to be tiny in which case the module is just adding overhead.

@AndrewScheidecker
Copy link

I'm not opposed to adding this function, but I think a better API for feature testing would be a WebAssembly.hasFeature function. It avoids the problems that the has_feature opcode had by avoiding any support for parsing modules that use unsupported features, and seems much more efficient than downloading and parsing modules (even if they're tiny) to test whether a feature is supported.

@lukewagner
Copy link
Member Author

We've definitely spent some time discussing the "has feature X" family of solutions and the problem is precisely defining the set of X (and choosing the level of granularity). We've also gotten advice from people working on other web standards not to go down this route and instead support the existing JS pattern of testing whether representative things exist.

To feature test with validate, one doesn't have to download anything; as the modules should only be a few bytes each, one can simply construct the code from an array literal: WebAssembly.validate(Int32Array.of(0x6d736100, 0x1))

@taisel
Copy link

taisel commented Aug 6, 2016

The overhead of feature testing in runtime can be elided though, right?

@AndrewScheidecker
Copy link

We've definitely spent some time discussing the "has feature X" family of solutions and the problem is precisely defining the set of X (and choosing the level of granularity). We've also gotten advice from people working on other web standards not to go down this route and instead support the existing JS pattern of testing whether representative things exist.

Somebody has to come up with discrete features to test for, it's just a question of whether it's the application/toolchain or WebAssembly.

To feature test with validate, one doesn't have to download anything; as the modules should only be a few bytes each, one can simply construct the code from an array literal: WebAssembly.validate(Int32Array.of(0x6d736100, 0x1))

Cool, that does mitigate my concerns about performance.

@lukewagner
Copy link
Member Author

@taisel Yes, I'm assuming all the validate tests would happen at load time where they'd potentially block initiating the fetch of the module bytes, which means they lie on the critical path. During page load, an event loop can get bogged down by a number of expensive operations so the latency of promise resolution could be exacerbated.

Somebody has to come up with discrete features to test for, it's just a question of whether it's the
application/toolchain or WebAssembly

Agreed, and I think the toolchain is the much better place since it knows exactly the granularity it needs.

then a `TypeError` is thrown.

Otherwise, this function performs *validation* as defined by the WebAssembly
spec and returns `true` if validation succeeded, `false` if validation failed.
Copy link
Member

Choose a reason for hiding this comment

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

Hm, just returning a Boolean means that this function provides less information than compile. I kind of expect that there might be users who would like to get a handle on the concrete error message, even if it is just to display a diagnostic.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking here is that compile isn't supposed to fail and, if it does, we can generate a big expensive error report (maybe eventually pretty-printing the text leading up to the validation failure, like we do in JS). With validate, we do expect it to fail a lot (hence return value, not throwing), so I thought this should be super-fast and lightweight by design. It seems like, if you are debugging bad compiler output, you have a wide variety of offline validator tools or compile at your disposal and so this might simply not be a use case for validate.

Copy link
Member

Choose a reason for hiding this comment

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

I'm more thinking of cases where an app wants to log some reason, or wants to say "Can't run because ...". I agree that is not a practice we would recommend, but it's the web... That said, I don't feel strongly about it, so LGTM anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right such telemetry would be important, but I think if you expect something does validate, you won't call validate, you'll just compile it, .catch() any error, and report that back like, well, any other unexpected error.

@lukewagner
Copy link
Member Author

Merging based on lgtm above.

@lukewagner lukewagner merged commit 672844f into master Aug 29, 2016
@lukewagner lukewagner deleted the validate-module branch August 29, 2016 16:31
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

6 participants