-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Remove offset-converter
run dependency. NFC
#14757
Conversation
c8307db
to
00096d9
Compare
a8017bd
to
37c46db
Compare
1de42f6
to
63361f7
Compare
offset-converter
run dependency when actually needed. NFCoffset-converter
run dependency. NFC
9ba935d
to
f41191a
Compare
src/preamble.js
Outdated
return WebAssembly.instantiate(binary, info); | ||
}).then(function (instance) { | ||
#if USE_OFFSET_CONVERTER | ||
// See comment below in instantiateAsync |
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.
Which comment does this refer to? If it's for We need the wasm binary for the offset converter ..
then I'm not sure how it's related?
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 one on line 1139: When using the offset converter, we must interpose here.
.
Basically we can't call receiver
before we have assigned wasmOffsetConverter
.. for the same reason as on line 1139. I believe you fixed this in #14611 for the normal instantiateAsync
but not for the fallback instantiateArrayBuffer
.
f41191a
to
1bc8d45
Compare
Following up on the change made in #14611, this change forces the `WasmOffsetConverter` before calling receiveInstantiationResult in both code paths (instantiateArrayBuffer and instantiateAsync). With that done (along with #14755) there are no more valid use cases for to block execution based on the absence of `WasmOffsetConverter`. By the time the instance is received it is always defined.
1bc8d45
to
f3d62d6
Compare
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 see, makes sense. Yes, I think that's right.
Co-authored-by: Alon Zakai <[email protected]>
Remove
offset-converter
run dependency. NFCFollowing up on the change made in #14611, this change forces the
WasmOffsetConverter
before calling receiveInstantiationResult in bothcode paths (instantiateArrayBuffer and instantiateAsync).
With that done (along with #14755) there are no more valid use cases for
to block execution based on the absence of
WasmOffsetConverter
. Bythe time the instance is received it is always defined.