1860066 - Wasm{Array,Struct}Object::obj_trace: don't pass nullptr to TraceManuallyBarrieredEdge

Status: RESOLVED WONTFIX

(Core :: JavaScript: WebAssembly, enhancement, P2)

Description

[Julian Seward [:jseward]]

The Kotlin benchmark subset microBenchmarks.ClassBaselineBenchmark appears
to do quite a lot of GC (scanning) of structs/arrays whose payload values are
ref-typed, but are mostly null. These null values are all carefully passed to
TraceManuallyBarrieredEdge, which (I think) ignores them. But that's
expensive.

This trivial patch merely skips the call if the passed value is null. It gets
the following results (Intel Core i5 1135G7, Linux):

In microBenchmarks.ClassBaselineBenchmark

                       ------- BEFORE -------         ------- AFTER -------
allocateArray          0.00261357 ms/op ±0.11%     0.00241290 ms/op ±0.086%
allocateArrayAndFill      9.84522 ms/op ±0.54%        9.70223 ms/op ±0.53%
allocateList           0.00273871 ms/op ±0.089%    0.00246473 ms/op ±0.090%
allocateListAndFill       10.2361 ms/op ±0.75%        10.0903 ms/op ±0.42%
allocateListAndWrite     0.280480 ms/op ±0.14%       0.281488 ms/op ±0.16%
consume                   13.4023 ms/op ±0.44%        13.7674 ms/op ±0.43%
consumeField            0.0485082 ms/op ±4.2%       0.0489072 ms/op ±4.4%

Not sure what's with the consume subtest. I couldn't see any reason for the
difference. It's a nuisance that these benchmarks run for a fixed amount of
time rather than a fixed amount of work, since that makes it difficult to be
confident, when comparing before-vs-after profiles, that they really ran
exactly the same workload.

Comment 1

[Julian Seward [:jseward]]

Attachment: bug1860066-dont-trace-nulls.diff-1

Comment 2

[Ryan Hunt [:rhunt]]

It's a bit unfortunate this is necessary, but seems like a quick win. Do you want to put it up for review?

Comment 3

[Ryan Hunt [:rhunt]]

I'm going to mark this as subsumed by PGO.