Use proxy objects for MEMORY64 HEAP access over 4Gb #19737

dakenf · 2023-06-28T17:38:56Z

Currently HEAP arrays fail to create when memory size is > 4gb so proxy objets for HEAP(U)8 are created when allocated memory is >4gb and for HEAP(U)16 when >8gb.

Solution overrides TypedArray set, subarray, fill, copyWithin, and slice methods. As well as direct property access using memory pointers. It creates N TypedArray views with maximum of 4 * 1024 * 1024 * 1024 - 2 elements for each heap view.

Also, test_memory64_proxies test added to check correctness of the implementation.

See https://bugs.chromium.org/p/v8/issues/detail?id=4153

sbc100 · 2023-06-28T18:49:30Z

Thanks for working on this! I've been meaning to do something very similar myself but you have likely saved me a bunch of work here!

dakenf · 2023-06-28T18:52:45Z

Sure! I will fix all the failing tests soon

src/preamble.js

src/library_webgpu.js

…GB is set This extends the use automatically-generated JS wrappers to handle conversion of incoming i32 pointer to u32 JS numbers. This is only needed when CAN_ADDRESS_2GB is set. See #19737

dakenf · 2023-06-29T18:11:55Z

I've fixed tests and now proxies are created only if allocated memory is > 4gb
Will remove fixPointer once #19740 is merged (would like to have a fully working version for now)
Let me know if some other changes are needed

Also, memory growth does not work even if initial memory is >4gb, looks like chrome issue. Now going to try make it working with ASYNCIFY=2 and threads. First one throws bigint to number error somewhere, and second gives linker error. Guess the latter would require some deep digging

…GB is set This extends the use automatically-generated JS wrappers to handle conversion of incoming i32 pointer to u32 JS numbers. This is only needed when CAN_ADDRESS_2GB is set. See #19737

…GB is set (#19740) This extends the use automatically-generated JS wrappers to handle conversion of incoming i32 pointer to u32 JS numbers. This is only needed when CAN_ADDRESS_2GB is set. See #19737

juj · 2023-07-03T11:47:31Z

Currently HEAP arrays fail to create when memory size is > 4gb

Isn't this something that browsers will need to fix as part of adding support for wasm64?

dakenf · 2023-07-03T12:12:15Z

Isn't this something that browsers will need to fix as part of adding support for wasm64?

Yes. But who knows when that would happen. The workaround allows using >4gb now and can be removed without modifying any other code once full 64bit support is rolled out in browsers. It does not affect anything when <=4gb memory is used

sbc100 · 2023-07-05T14:33:39Z

emcc.py

@@ -3728,7 +3728,8 @@ def phase_binaryen(target, options, wasm_target):
    # >=2GB heap support requires pointers in JS to be unsigned. rather than
    # require all pointers to be unsigned by default, which increases code size
    # a little, keep them signed, and just unsign them here if we need that.
-    if settings.CAN_ADDRESS_2GB:


This line can now be reverted since CAN_ADDRESS_2GB is never set when MEMORY64 is set: See #19755

src/library_strings.js

sbc100 · 2023-07-05T14:38:17Z

src/preamble.js

+  var maxArraySize = Math.min(b.byteLength, 4 * 1024 * 1024 * 1024 - 8);
+  var proxyHandler = (type, heapBlock) => ({
+    heapBlock,
+    copyWithin (target, start, end) {


Our JS style has no space between function names and their arguments

src/runtime_safe_heap.js

sbc100 · 2023-07-05T14:40:38Z

src/preamble.js

+    copyWithin (target, start, end) {
+      target = fixPointer(target)
+      start = fixPointer(start)
+      end = fixPointer(end)


I'm hoping you can simply remove fixPointer since all incoming pointers should already i53 (and unsigned). See #19740.

src/preamble.js

sbc100 · 2023-07-05T14:45:40Z

Can you add a comment to this new code and include a link to the upstream bug: https://bugs.chromium.org/p/v8/issues/detail?id=4153

dakenf · 2023-07-05T14:50:53Z

Can you add a comment to this new code and include a link to the upstream bug: https://bugs.chromium.org/p/v8/issues/detail?id=4153

Sure. I've made a fix for ASYNCIFY=1 and MEMORY64=1. Should I include it into this PR or open a separate one?

sbc100 · 2023-07-10T22:43:25Z

lgtm

I re-titled this PR for clarity.

sbc100 · 2023-07-11T00:39:54Z

test/test_browser.py

    # test that growth doesn't go beyond 2GB without the max being set for that,
    # and that we can catch an allocation failure exception for that
    self.emcc_args += ['-O2', '-sALLOW_MEMORY_GROWTH', '-sMAXIMUM_MEMORY=2GB']
    self.do_run_in_out_file_test('browser', 'test_2GB_fail.cpp')

  @no_firefox('no 4GB support yet')
  @also_with_wasm64
-  @requires_v8


Why this? In general if we can test on the command line I think we should prefer to do that. Browser tests tend to be more flaky and difficult to work with (for example we don't get to see the stdout in CI).

It fails with a weird assertion error which i cannot reproduce locally. Neither in command line, nor in the browser. Maybe because of actual out of memory issue?
Will dig a bit more once this test run finishes

dakenf · 2023-07-11T18:29:49Z

I have disabled test_zzz_zzz_4gb_fail_wasm64 because it fails during stack overflow check

RuntimeError: Aborted(Assertion failed)
    at abort (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:13321)
    at assert (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:4639)
    at ptrToString (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:20015)
    at checkStackCookie (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:9970)
    at handleException (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:24645)
    at callMain (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:33035)
    at doRun (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:33424)
    at run (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:33592)
    at runCaller (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:32614)
    at removeRunDependency (/tmp/emscripten_test_browser_grkv22q_/test_4GB_fail.js:1:13176)

in checkStackCookie function _emscripten_stack_get_end returns undefined. Which is later passed to ptrToString and it fails converting undefined to a string.

I can add if (typeof max === 'undefined) max = 0 here but it feels wrong

function checkStackCookie() {
#if !MINIMAL_RUNTIME
  if (ABORT) return;
#endif
  var max = _emscripten_stack_get_end();
#if RUNTIME_DEBUG
  dbg(`checkStackCookie: ${ptrToString(max)}`);
#endif
  // See writeStackCookie().
  if (max == 0) {
    max += 4;
  }
...

sbc100 · 2023-07-11T18:43:32Z

Interesting... _emscripten_stack_get_end is a native function and it should never return undefined. Something strange must be going on.

dakenf · 2023-07-11T18:52:20Z

Interesting... _emscripten_stack_get_end is a native function and it should never return undefined. Something strange must be going on.

Indeed. And it runs successfully on my machine. So i'd guess it should be a hardware out of memory problem which gets the runtime mad

dakenf · 2023-07-18T19:25:49Z

I've fixed tests from main branch here. LLVM update changed something in output size

sbc100

lgtm if tests pass!

sbc100 · 2023-07-26T14:52:15Z

test/test_browser.py

-    #      means we don't compete for memory with anything else (and run it
-    #      at the very very end, to reduce the risk of it OOM-killing the
-    #      browser).
-


Restore this comment?

sbc100 · 2023-07-26T14:52:37Z

test/test_browser.py

@@ -5371,14 +5371,7 @@ def test_wasm_worker_proxied_function(self):
    self.btest(test_file('wasm_worker/proxied_function.c'), expected='0', args=['--js-library', test_file('wasm_worker/proxied_function.js'), '-sWASM_WORKERS', '-sASSERTIONS=0'])

  @no_firefox('no 4GB support yet')
-  @requires_v8


Is this needed?

sbc100 · 2023-07-26T14:53:04Z

test/test_other.py

  def test_quick_exit(self):
    self.do_other_test('test_quick_exit.c')
+
+


Just one newline between methods in python

sbc100 · 2023-07-26T14:53:47Z

test/test_other.py

+                      '-Wno-experimental',
+                      '--extern-post-js', 'post.js'])
+    self.run_js('a.out.js')
+


Remove extra newline

sbc100 · 2023-07-26T14:54:36Z

test/test_other.py

+  @requires_node_canary
+  def test_memory64_proxies(self):
+    create_file('post.js', r'''
+  addOnPostRun(() => {


Maybe put this in its own file? (test/other/test_memory64_proxies.js?)

dakenf · 2023-07-26T15:26:15Z

@sbc100 All done. Can you please also guide me on this one? WebAssembly/memory64#39

I've created a same proposal in WebAssembly/threads but the team has closed it as it should be part of the Memory64 spec. Currently it's not possible to create WebAssembly.Memory from JS with 64bit limits. And it's easy to fix in V8 source (just a couple of ifs). But I don't think my PR in V8 would be merged without a spec change as there's a comment like "update this when js spec is updated"

sbc100 · 2023-07-26T16:34:31Z

Can you try reverting all the test/code_size changes? I think that should help with a bunch of the current failures.

dakenf · 2023-07-26T16:39:11Z

Can you try reverting all the test/code_size changes? I think that should help with a bunch of the current failures.

Yup. Should be good now

dakenf · 2023-07-27T13:53:59Z

Thanks! I's been a long way

juj · 2023-08-02T06:30:15Z

This PR only added polyfills for HEAP8 and HEAP16, but not HEAP32 or HEAP64. I don't see that being discussed in above, and wonder why that was the case?

sbc100 · 2023-08-02T15:18:18Z

This PR only added polyfills for HEAP8 and HEAP16, but not HEAP32 or HEAP64. I don't see that being discussed in above, and wonder why that was the case?

The reason is that the chromium issue/limit is on the number of elements in the view not the byte length. So even without this fix we can create HEAP32 views that can address 16gb. I seem to remember there are other implementation limits in chrome that will get hit before we get to 16gb. I'm hoping that by the we want to allow for >16gb memories this polyfill will no longer be needed.

MEMORY64 HEAP proxy objects

1440466

sbc100 reviewed Jun 28, 2023

View reviewed changes

src/preamble.js Outdated Show resolved Hide resolved

src/preamble.js Outdated Show resolved Hide resolved

src/library_webgpu.js Outdated Show resolved Hide resolved

sbc100 mentioned this pull request Jun 28, 2023

Automatically convert i32 pointer args to unsigned when CAN_ADDRESS_2GB is set #19740

Merged

Fixed tests, conditional proxy creation

dfdcf9b

Merge branch 'main' into main

3d832fb

dakenf added 2 commits July 2, 2023 13:53

Merge branch 'main' into main

db6bf73

Removed fixPtr function

799dc73

sbc100 reviewed Jul 5, 2023

View reviewed changes

sbc100 changed the title ~~MEMORY64 HEAP proxy objects~~ Use proxy objects for MEMORY64 HEAP access over 4Gb Jul 10, 2023

Run 4gb memory tests in the browser instead of d8

c0d9a76

sbc100 reviewed Jul 11, 2023

View reviewed changes

Disabled test_zzz_zzz_4gb_fail_wasm64

34b85dc

sbc100 mentioned this pull request Jul 12, 2023

How does emscripten generate 32-bit and 64-bit wasm #19833

Closed

dakenf added 4 commits July 13, 2023 02:13

Merge branch 'main' into main

52516b1

Removed console.log

ed8bccf

Merge branch 'main' into main

d1fd445

Upstream test fixes

a758166

Merge branch 'main' into main

b4c6d52

sbc100 approved these changes Jul 26, 2023

View reviewed changes

Test fixes

21d06c9

Reverted test size changes

35f06f0

sbc100 merged commit 233e004 into emscripten-core:main Jul 27, 2023

sbc100 mentioned this pull request Aug 4, 2023

Implement macros {{{ ptrToIdx() }}}, {{{ convertPtrToIdx() }}}, {{{ idxToPtr() }}} #19289

Open

sbc100 mentioned this pull request Aug 21, 2023

Embind problems with val::as<T>() in wasm32 with MAXIMUM_MEMORY=4GB #19968

Closed

jspanchu mentioned this pull request Oct 23, 2023

wasm64: bufferSubData fails after heap grows beyond 4 GB #20517

Open

sbc100 mentioned this pull request Oct 27, 2023

wasm64: We can't yet use wasm memories > 4Gb #19512

Closed

Zambonilli mentioned this pull request Dec 30, 2023

Upgrade diffusers.js for gpu support Zambonilli/genai-gamelist#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use proxy objects for MEMORY64 HEAP access over 4Gb #19737

Use proxy objects for MEMORY64 HEAP access over 4Gb #19737

dakenf commented Jun 28, 2023 •

edited

Loading

sbc100 commented Jun 28, 2023

dakenf commented Jun 28, 2023

dakenf commented Jun 29, 2023

juj commented Jul 3, 2023

dakenf commented Jul 3, 2023

sbc100 Jul 5, 2023

sbc100 Jul 5, 2023

sbc100 Jul 5, 2023

sbc100 commented Jul 5, 2023

dakenf commented Jul 5, 2023

sbc100 commented Jul 10, 2023

sbc100 Jul 11, 2023

dakenf Jul 11, 2023

dakenf commented Jul 11, 2023

sbc100 commented Jul 11, 2023

dakenf commented Jul 11, 2023

dakenf commented Jul 18, 2023

sbc100 left a comment

sbc100 Jul 26, 2023

sbc100 Jul 26, 2023

sbc100 Jul 26, 2023

sbc100 Jul 26, 2023

sbc100 Jul 26, 2023

dakenf commented Jul 26, 2023

sbc100 commented Jul 26, 2023

dakenf commented Jul 26, 2023

dakenf commented Jul 27, 2023

juj commented Aug 2, 2023

sbc100 commented Aug 2, 2023

		def test_quick_exit(self):
		self.do_other_test('test_quick_exit.c')

Use proxy objects for MEMORY64 HEAP access over 4Gb #19737

Use proxy objects for MEMORY64 HEAP access over 4Gb #19737

Conversation

dakenf commented Jun 28, 2023 • edited Loading

sbc100 commented Jun 28, 2023

dakenf commented Jun 28, 2023

dakenf commented Jun 29, 2023

juj commented Jul 3, 2023

dakenf commented Jul 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbc100 commented Jul 5, 2023

dakenf commented Jul 5, 2023

sbc100 commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dakenf commented Jul 11, 2023

sbc100 commented Jul 11, 2023

dakenf commented Jul 11, 2023

dakenf commented Jul 18, 2023

sbc100 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dakenf commented Jul 26, 2023

sbc100 commented Jul 26, 2023

dakenf commented Jul 26, 2023

dakenf commented Jul 27, 2023

juj commented Aug 2, 2023

sbc100 commented Aug 2, 2023

dakenf commented Jun 28, 2023 •

edited

Loading