1474914 - Poor performance of Array.prototype.reverse() compared to other JS engines

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[TheQwertiest]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20180711100118

Steps to reproduce:

1. Create a big dense array of numbers (e.g. 30k elements).
2. Run array.reverse() in a loop to measure it's performance (e.g. 1k loops).

Or

Run the following fiddle:
https://jsfiddle.net/c42vd3m9/1014/


Actual results:

Result (vary +-2ms, but relative positions stay the same):
Browsers:
- IE11: 12ms
- Firefox 63.0a1 (2018-07-11): 38ms
- Firefox 63.0a1 (2018-07-01): 30ms
- Opera 53.0.2907.48: 36ms
- Vivaldi 1.12.955.36: 27ms

Engines:
- Embedded WSH (uses IE11 engine): 12ms
- Embedded SpiderMonkey ESR60: 38ms


Expected results:

Expected IE to be the worst...

Comment 1

[TheQwertiest]

Note: SpiderMonkey for embedding was built as 32bit dll

Comment 2

[TheQwertiest]

Status update:
Updated repro link (with working console.log): https://jsfiddle.net/v04noa1p/

Browsers:

• IE Edge: 12ms
• Firefox 70.0.1: 35ms
• Chrome 78.0.3904.108: 50ms (so slow! muahahaha!)

In other words, FF is still much slower than IE.

Comment 3

[André Bargull [:anba]]

This takes longer because the complete array is traversed when emitting post-write barriers in NativeObject::elementsRangeWriteBarrierPost when called from NativeObject::reverseDenseElementsNoPreBarrier. So it should be faster when the array is filled with gc-things (objects, strings) from the nursery instead of primitives like numbers, because then the loop in elementsRangeWriteBarrierPost doesn't check each array element...

But do we actually need to emit a post-write barrier when the array itself is in the nursery (that's the situation in the test case)? When reading gc::StoreBuffer::put it looks like nursery things are ignored anyway (cf. SlotsEdge::maybeInRememberedSet).

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1474914: Skip elementsRangeWriteBarrierPost for nursery objects. r=jonco!

StoreBuffer::putSlot when called with a nursery object as its obj parameter
is a no-op, because StoreBuffer::put is a no-op when Edge::maybeInRememberedSet
return false, which, in the case of SlotsEdge, happens when SlotsEdge::object()
is in the nursery. This enables us to skip the linear traversal of the elements
array in elementsRangeWriteBarrierPost when the current object is in the nursery.

Comment 5

[Jon Coppeard (:jonco)]

This is a great catch.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7074213657a3
Skip elementsRangeWriteBarrierPost for nursery objects. r=jonco

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7074213657a3

Comment 8

[TheQwertiest]

For posterity, on the same fiddle:
FF 89: 8ms
Chrome: 20ms
(chromium) Edge (no surprise): 20ms