Closed Bug 1392234 Opened 7 years ago Closed 4 years ago

Allow ArrayBuffers to grow past 2GB (in 64-bit browser processes)

Categories

(Core :: JavaScript Engine, enhancement, P2)

x86_64
All
enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
firefox57 --- fix-optional

People

(Reporter: jujjyl, Unassigned)

References

(Blocks 1 open bug)

Details

A couple of times now I have gotten questions about why JavaScript typed arrays max out at signed 32-bit int limit (2GB-1byte), and have been unable to answer this. Trying to track down the spec call flow for any notable restrictions, starting from http://www.ecma-international.org/ecma-262/6.0/#sec-%typedarray%-length and reaching to http://www.ecma-international.org/ecma-262/6.0/#sec-createbytedatablock I am unable to find a spec reason at least for forbidding "new ArrayBuffer(4*1024*1024*1024-1);" from working. Does anyone know if the signed int32 restriction for ArrayBuffer sizes is Firefox's own limitation, or is there some specific reason that we can't go higher? Or would it be possible to just change Firefox to expand the max size to be 4GB-1byte? There are some applications in image processing, video codecs, OpenCV and games that would benefit from being able to allocate WebAssembly heaps of size (4GB - 64KB) (Wasm page size being 64KB). Until Wasm64 later in the future, this would give a bit of a breather to generic computation applications that require possibly lots of memory.
I believe this is Firefox's limitation. I don't know if we have any limit on object size per se but the ArrayBuffer code uses INT32_MAX as a limit for legal offsets and length, here and there.
If we do this we should audit all of our TypedArray/ArrayBuffer length/offset code... C++ implicit signed/unsigned conversion is a real footgun here.
(In reply to Jan de Mooij [:jandem] from comment #2) > If we do this we should audit all of our TypedArray/ArrayBuffer > length/offset code... C++ implicit signed/unsigned conversion is a real > footgun here. Comforting to know if it's only this. That is, I was wondering if there might be some asm codegen related considerations, where we might have to adjust some generated asm instructions that would be doing some assumptions. Does anything of this sorts come to mind?
For starters, ArrayBuffer stores its length in a Value slot, and assumes Int32. I expect this probably aids self-hosting the ArrayBuffer methods in various ways, since a value > INT32_MAX would end up being represented as a double. Run-time type analysis might make the representation issue a non-problem, I don't know. I see Ion inlines ArrayBuffer.p.length and assumes an Int32 type now; that would have to change.
And ditto the path for the length of a TypedArray assumes int32 all the way; the length is used extensively for self-hosting of course. Not obvious to me how changing this to a more challenging type would affect anything. And again this is stored in a Value slot in the TypedArray object.
If we plan for the long term, since ABs are spec'ed to be able to be bigger than 4gb, limited only by the range of doubles' ability to precisely represent integers (so 2^53), we could change the length to a Value, and rely on the usual int32/double JIT specialization to not regress perf too much. If the internal AB::length() returned a Value, that would avoid any implicit cast madness.
Looks like V8 is doing this: https://bugs.chromium.org/p/v8/issues/detail?id=7881 Maybe we should prioritize fixing this to avoid a new crop of wasm apps that OOM in FF. Generalizing the current int32_t to a uint32_t is probably a shorter path than generalizing to something bigger like I was suggesting earlier.
I missed that one, I was watching another V8 bug with the same subject... Looks like V8 keeps the 2^31-1 limit on 32-bit but boosts it to the full 2^53-1 range on 64-bit; of course, with wasm being 32-bit we don't need more than 4GB currently, but given the amount of vetting we'll need, I don't know... There's some appeal in stopping at 2^32 but in practice anything above 2^31-1 will easily turn into a double anyway, right? Lots of self-hosted code. I'm wondering if it is a viable halfway solution to keep the 2^31-1 limit in the representation for TypedArray but to allow wasm memories (and I guess ArrayBuffers) to be larger, and to handle larger accesses via TA along the OOB path. That is, wasm runs at full speed up to the AB size but JS will bail to the interpreter and the interpreter will fixup accesses in the top 2GB. It's a performance cliff for sure, but entails less JIT and representation work. On the other hand there are uncertainties about hot well this works with our self-hosted primitives and so on.
Summary: 4GB typed arrays for 64-bit browser processes? → 4GB ArrayBuffers for 64-bit browser processes?

Google is now pushing to have the 2GB max memory size in wasm raised to 4GB: https://github.com/WebAssembly/spec/issues/1116. I've made the point on that ticket that this represents a compatibility problem until SpiderMonkey can fix its code but I doubt that will sway the vote much. We're probably going to have to look into this sooner rather than later.

There's now some movement to move wasm beyond 32-bit memories, https://github.com/WebAssembly/design/issues/1325. This could happen quickly: it doesn't require an elaborate "wasm64" design, only larger memories and int64 addresses; it's a fairly local change. We should not lock ourselves into a situation where we stop at 4GB. We should probably allow for the range of an Index, ie, up to 2^53-1 bytes, even if it will remain impractical to allocate memories much beyond physical RAM, and RAM mostly maxes out at 16GB currently.

Summary: 4GB ArrayBuffers for 64-bit browser processes? → Allow ArrayBuffers to grow past 2GB (in 64-bit browser processes)
Priority: P3 → P2
Depends on: 1673557
Blocks: 1673619
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Assignee: lhansen → nobody
Status: ASSIGNED → NEW

Update: this is now available in Nightly when setting javascript.options.large_arraybuffers to true (requires a browser restart).

Depends on: 1703505

Pref is on by default starting in FF89.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.