[threads][test][js-api] Add tests for growing shared memory#248
[threads][test][js-api] Add tests for growing shared memory#248backes wants to merge 1 commit intoWebAssembly:mainfrom
Conversation
Add a few first (!) js-api test to this repo. In particular: - Growing a non-shared memory always detaches the old buffer and allocates a fresh one (as before). - Growing a shared memory only refreshes the buffer if the delta in pages is > 0. - Growing a shared memory has no effect on unrelated shared memories. Note that V8 didn't implement this correctly until https://crrev.com/c/7660970, see https://crbug.com/492128992.
Shared ArrayBuffers need replacement whenever their backing store grows, as they do not update their 'byteLength' automatically. This was previously done unconditionally for all shared memories on any growth (via 'UpdateSharedWasmMemoryObjects'). This CL changes it so only those 'JSArrayBuffer's are cleared (and thus refreshed on their next access) that are actually smaller than their backing store. Note that for non-shared memory, 'grow(0)' still detaches and replaces the buffer, as mandated by the current JS API spec. Shared memory follows the Threads proposal which specifically preserves identity when the length doesn't change. A respective spec test is added in WebAssembly/threads#248. R=jkummerow@chromium.org Bug: 492128992 Change-Id: I76a0f4ce23c98eb73c3a74c36fb0795f08e9ffa4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7660970 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/main@{#105812}
|
I don't see where the spec says to check for delta > 0 before refreshing the buffer. https://webassembly.github.io/threads/js-api/index.html#grow-the-memory-buffer |
|
Oh, I was basing this on this algorithm in the overview: https://github.com/WebAssembly/threads/blob/main/proposals/threads/Overview.md#webassemblymemoryprototypebuffer But now I see that this is totally different to the growing algorithm and the |
|
Urgh, sorry about this divergence. Experimentally, what do engines do right now? |
|
It looks like all browsers implement the algorithm in the overview, not the spec :D I attached an HTML page for testing and a little python script for serving the HTML file with the right COOP and COEP headers (vibe coded): Results: (Note that Chrome 147 behaved differently and refreshed all ArrayBuffers for all shared Wasm memories in the isolate; this was fixed by https://crrev.com/c/7660970.) |
|
I opened a PR to adapt the JS-API spec to match what's described in the overview: #255 |
3 participants
Add a few first (!) js-api test to this repo. In particular:
Note that V8 didn't implement this correctly until https://crrev.com/c/7660970, see https://crbug.com/492128992.