Use singleton type only for big typed arrays created with an existing buffer

RESOLVED FIXED in Firefox 60

Status

()

RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla60
Points:
---

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 months ago
Created attachment 8950157 [details] [diff] [review]
Patch

We have an optimization to give singleton type to TypedArrays with byteLength >= 10 MB. This was added to optimize asm.js/Mandreel type code better in Ion, but in practice this optimization can easily backfire because we quickly reach TypeSet limits when allocating many singleton arrays. mraleph ran into a SpiderMonkey perf cliff caused by this and I've seen it regress perf before.

One easy short-term fix is to only use this optimization for typed arrays created with an existing buffer, as that's the pattern used by Mandreel/asm.js code. That also makes it easier for people to opt-out of this optimization.

Longer term I think we should consider removing this optimization (because wasm) but as a first step restricting it seems reasonable.
Attachment #8950157 - Flags: review?(bhackett1024)
(Assignee)

Comment 1

10 months ago
Created attachment 8950172 [details] [diff] [review]
Patch
Attachment #8950157 - Attachment is obsolete: true
Attachment #8950157 - Flags: review?(bhackett1024)
Attachment #8950172 - Flags: review?(bhackett1024)
Comment on attachment 8950172 [details] [diff] [review]
Patch

Review of attachment 8950172 [details] [diff] [review]:
-----------------------------------------------------------------

It would be fine to remove this optimization entirely I think.
Attachment #8950172 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 3

10 months ago
(In reply to Brian Hackett (:bhackett) from comment #2)
> It would be fine to remove this optimization entirely I think.

This will regress Octane-mandreel a bit (~4000 points IIRC). Maybe we should take the hit and investigate but for now I'll just land this patch.

Comment 4

10 months ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7519d21db96
Use singleton type only for big typed arrays created with an existing buffer. r=bhackett

Comment 5

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e7519d21db96
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.