1931959 - Wasm array tracing can block interrupt handling for too long

Status: NEW

(Core :: JavaScript: WebAssembly, task, P3)

Description

[Ryan Hunt [:rhunt]]

I received a report from Christian Holler that very large wasm arrays are not playing well with timeouts. In this example, a really large wasm array is created and 1 second timeout is set. However the test case can take 11 seconds to timeout.

mod = new WebAssembly.Module(wasmTextToBinary(`
(module
  (type (;0;) (sub (array eqref)))
  (type (;1;) (sub (array eqref)))
  (type (;2;) (sub (struct (field f32))))
  (type (;3;) (array i32))
  (type (;4;) (func (param anyref anyref anyref) (result f64)))
  (type (;5;) (array (mut i16)))
  (type (;6;) (func (param externref) (result (ref extern))))
  (type (;7;) (func (param externref) (result i32)))
  (type (;8;) (func (param i32) (result f64)))
  (import "fuzzGlobal" "g0" (global (;0;) externref))
  (import "fuzzFunc" "f1" (func (;0;) (type 4)))
  (import "wasm:js-string" "cast" (func (;1;) (type 6)))
  (import "wasm:js-string" "length" (func (;2;) (type 7)))
  (table (;0;) i64 30 255 funcref)
  (memory (;0;) i64 11 13 shared)
  (global (;1;) eqref ref.null eq)
  (export "g1" (global 0))
  (export "t1" (table 0))
  (export "m1" (memory 0))
  (export "main" (func $main))
  (func $main (;3;) (type 8) (param i32) (result f64)
    f32.const inf (;=inf;)
    struct.new 2
    local.get 0
    array.new_default 1
    struct.new_default 2
    local.get 0
    array.new 1
    call 0
  )
)
`), { builtins: ["js-string"] });

inst = new WebAssembly.Instance(mod, { 'fuzzGlobal' : { 'g0' : '' }, 'fuzzFunc' : { 'f1' : function() {} }, });

timeout(1);

inst.exports["main"](Math.sqrt(2**53));

Here's a local profile of a debug build [2]. Nearly all of the time is spent in a majorGC inside a single trace function for WasmArrayObject::obj_trace.

I wonder if we need to check an interrupt flag or SliceBudget while tracing. I see that JS array builtins sometimes check the interrupt flags, and GC code will check a SliceBudget, but never within a trace function.

[2] https://share.firefox.dev/3ZgdxY7

Comment 1

[Ryan Hunt [:rhunt]]

Jon, do you know if it makes sense to check a SliceBudget within a trace function? I don't see a nice way to do that, but don't know of any alternative here.

Comment 2

[Ryan Hunt [:rhunt]]

It looks like marking has a manual scanning loop for JS arrays that checks slice budgets manually. I bet we could add a similar case for wasm arrays. [1]

[1] https://searchfox.org/mozilla-central/rev/6050bf4eca89956c9d91bfd89fa59294ae32a689/js/src/gc/Marking.cpp#1578

Comment 3

[Ryan Hunt [:rhunt]]

Extending SlotsOrElementsRange to also support wasm arrays reduces the total time in the test case from 11 seconds to 5.5 seconds. The other large area of non-interruptible code seems to be tenuring.

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: WIP: Bug 1931959 - WIP - mark wasm arrays inline

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Ryan Hunt [:rhunt] from comment #1)
There's no way to check the slice budget from the trace function at the moment, but it's possible we could arrange that you could get it from the tracer.

It might be a good idea to do this because we have the same issue for JS Map/Set objects currently (although I expect they cannot grow as large as wasm arrays).

The patch is a neat idea. Any idea how this affects GC marking speed in general though?

Comment 6

[Jon Coppeard (:jonco)]

(In reply to Ryan Hunt [:rhunt] from comment #3)
Tenuring is not interruptible. We probably shouldn't support allocating really large arrays in the nursery for this reason.

Comment 7

[Ryan Hunt [:rhunt]]

I don't know how it affects GC marking speed in general, do you know a good test case to measure that?

Getting the slice budget in the trace function is half the problem, I think, because we also need to signal to the tracer that the trace function did not complete and needs to be called again.

Good point about tenuring. We should be able to disallow that wasm GC arrays when they're really big.

Comment 8

[Jon Coppeard (:jonco)]

(In reply to Ryan Hunt [:rhunt] from comment #7)
Slice budget is possibly the easier half though as you also need to preserve some state between invocations so you know where to start marking again.

Probably we should handle tracing large containers separately from the trace function and leave that signature as is. There are only a few things that can be large enough to require this.