1751728 - Make Vector asserts into release asserts

Status: NEW

(Core :: MFBT, task)

Description

[Andrew McCreight [:mccr8]]

It would be nice if we could make at least the bounds checks for the MFBT Vectors into release asserts, like we have for nsTArray.

Comment 1

[Lars T Hansen [:lth]]

I'm a little concerned that this will negatively impact JS and wasm compile times, but I guess we'll see.

Comment 2

[Andrew McCreight [:mccr8]]

That is something to keep in mind, but release assert bounds checks were added to nsTArrays without any noticeable performance impact.

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Andrew McCreight [:mccr8] from comment #2)

That is something to keep in mind, but release assert bounds checks were added to nsTArrays without any noticeable performance impact.

JS engine code might be more performance-sensitive (on average) than other code in Gecko though...

Comment 4

[Andrew McCreight [:mccr8]]

SegmentedVector and Buffer also have the same kind of bounds check asserts.

Comment 5

[Andrew McCreight [:mccr8]]

What benchmarks should I check before trying to land this?

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Bug 1751728 - Make Vector bounds checks into release asserts.

Comment 7

[Jan de Mooij [:jandem]]

(In reply to Andrew McCreight [:mccr8] from comment #5)

What benchmarks should I check before trying to land this?

The ones in CI that come to mind are:

• speedometer
• jsshell-bench-*-sm
• matrix-react-bench
• sunspider
• wasm-godot-*

It would be nice to have a before vs after Perfherder URL.

Comment 8

[Lars T Hansen [:lth]]

We do a lot of vector operations during compilation in general. Wasm baseline compilation time will be interesting for sure, I forget whether the wasm-godot-* benchmarks display that or if baseline compilation time is reported separately. If need be I can also apply the patch locally and generate some timings on misc relevant hardware here.