Bug 220189 – [JSC] Update WebAssembly instance's exports object

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Update WebAssembly instance's exports object

[Yusuke Suzuki]

Created attachment 416835 [details]
Patch

[Yusuke Suzuki]

Created attachment 416836 [details]
Patch

[Alexey Shvayka]

Comment on attachment 416836 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416836&action=review

r=me

> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:512
> +    objectConstructorFreeze(globalObject, exportsObject);

Nice: the fast path for final objects will be taken.

[Alexey Shvayka]

Comment on attachment 416836 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416836&action=review

> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:513
> +    RETURN_IF_EXCEPTION(scope, void());

Is propertyName guaranteed to be non-index? putDirect() has an assert for that.
If it's never an index, we can just do `exportsObject->freeze(vm)` and remove this exception check.
Otherwise, we should use putDirectMaybeIndex() and scope.assertNoException() since SetIntegrityLevel can't throw per spec, nor it can return `false`.

[Yusuke Suzuki]

Comment on attachment 416836 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416836&action=review

Thanks!

>> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:513
>> +    RETURN_IF_EXCEPTION(scope, void());
> 
> Is propertyName guaranteed to be non-index? putDirect() has an assert for that.
> If it's never an index, we can just do `exportsObject->freeze(vm)` and remove this exception check.
> Otherwise, we should use putDirectMaybeIndex() and scope.assertNoException() since SetIntegrityLevel can't throw per spec, nor it can return `false`.

Oops, nice catch. I don't think this is guaranteed. I'll add some tests & use putDirectIndex if it is index.

[Yusuke Suzuki]

Committed r271112: <https://trac.webkit.org/changeset/271112>

[Radar WebKit Bug Importer]

<rdar://problem/72744953>

[Saam Barati]

This seems to have made richards-wasm 70% slower in JS2

[Saam Barati]

(In reply to Saam Barati from comment #8)
> This seems to have made richards-wasm 70% slower in JS2

The "runtime" score got 150% worse. Compile times are not effected

[Saam Barati]

My guess is this "breaks" the JS -> Wasm call fast path

[Yusuke Suzuki]

Looking.

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #11)
> Looking.

Found a fun issue. Will fix it soon in https://bugs.webkit.org/show_bug.cgi?id=220339