1932864 - Modified codepen demo spends a lot of time in Map, GCMinor, and Int32ToCStringWithBase

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Mayank Bansal]

The original demo has some parameters that you can play with.

Example1: Open the attached testcase

Map-heavy: https://share.firefox.dev/3V69QCR (4.8s)
Not Map heavy: https://share.firefox.dev/3V3HpFQ (12s)
We are faster than Chrome here. But filing this bug in case there is anything obvious we can improve. Else, please feel free to close.

Comment 1

[Mayank Bansal]

Attachment: Map-heavy.html

Comment 2

[Mayank Bansal]

Attachment: Non-map-heavy.html

Comment 3

[Mayank Bansal]

Profile from latest Nightly:
Map-heavy: https://share.firefox.dev/4f5J1Ww (4.2s)
Not map-heavy: https://share.firefox.dev/4fZ4fGX (8s)

Comment 4

[Iain Ireland [:iain]]

One thing I notice is that in the non-map case we spend ~20% of our time calling js::jit::IonGetPropertyIC::update, which implies that we're repeatedly hitting the fallback for a GetProp IC because we don't have CacheIR support for something.

Digging into this, it looks like we're repeatedly accessing element -1 of an array. In general it looks like handling negative indices is kind of tricky for us. TryAttachSparseElement doesn't apply, because the PropertyKey representation requires them to be represented as the atom "-1" instead of an int32.

It feels like we could do better here. Not entirely sure how, though.

The fact that we are already better than Chrome makes this a lower priority.

Comment 5

[Mayank Bansal]

Map-heavy: https://share.firefox.dev/4a81qRR
non-map-heavy: https://share.firefox.dev/3PrhBjw (8s)
(not much improvement from bug 1938518, as expected)

Comment 6

[Iain Ireland [:iain]]

Attachment: Bug 1932864: Treat negative integers as string ids r=jandem

This makes a microbenchmark accessing arr[-1] repeatedly 300x faster. The original "not-map-heavy" demo is noisy, but it looks like we're about 20% faster in my cut-down shell version, which is consistent with the observation that we're spending about 20% of our time in IonGetPropertyIC::update.

This leaves Double keys uncovered. I'm not sure how often they occur.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2bad00ee65ea
Treat negative integers as string ids r=jandem

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2bad00ee65ea

Comment 9

[Mayank Bansal]

Non-map-heavy: https://share.firefox.dev/4g5PdhM (~5s)
Map-heavy: https://share.firefox.dev/4heRecw (4.3s)

So we improved by about 40% from comment 5.

Comment 10

[Mayank Bansal]

Edited and obsolete.

Comment 11

[Mayank Bansal]

Improved Jetstream2-ai-astar by 6.2%

Comment 12

[Mayank Bansal]

Non-map-heavy: https://share.firefox.dev/3PNSXd2 (6.2s) - maybe slight regression or within the noise range for 1 run?
Map-heavy: https://share.firefox.dev/4jqWknX (4.7s)