Use unboxed objects by default

RESOLVED FIXED in Firefox 41

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

(Blocks: 1 bug)

Trunk
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8602263 [details] [diff] [review]
patch

Unboxed objects improve all our major benchmarks (octane by 2-3%) and don't regress any of the assorted tests on AWFY.  They're also a good memory savings when we can use them.  The attached patch turns them on by default.  I'll wait to land this until after the next merge and after bug 1161762 is fixed (the only remaining unboxed object crash I think.)

Since there isn't any need to control these with an about:config option I removed the unboxed objects option from RuntimeOptions and added a flag in JitOptions, as Hannes suggested in bug 1153266.  I hope that eventually this file will be generalized so that it isn't JIT specific, but for now this doesn't seem too bad and the weirdness with the different control mechanism for unboxed arrays will be fixed when they are also on by default.
Attachment #8602263 - Flags: review?(jdemooij)
Fuzzing team: when this patch lands, --unboxed-objects won't be available anymore, and will be replaced by the opposite --no-unboxed-objects, still useful to test (in differential mode at least).
Got it. Thanks for the headsup!
For the record, decoder asked on irc to mark open and potentially dangerous unboxed-objects bugs as s-s, but this list [0] doesn't show bugs without patches (note the unassigned one is actually about debugging, not about unboxed objects, although it triggers with --unboxed-objects).

[0] https://bugzilla.mozilla.org/buglist.cgi?emailreporter1=1&list_id=12235502&resolution=---&emailtype1=exact&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&longdesc=unboxed&email1=choller%40mozilla.com&component=JavaScript%20Engine&component=JavaScript%20Engine%3A%20JIT&component=JavaScript%3A%20GC&component=JavaScript%3A%20Internationalization%20API&component=JavaScript%3A%20Standard%20Library&longdesc_type=substring&product=Core
Comment on attachment 8602263 [details] [diff] [review]
patch

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

Nice!
Attachment #8602263 - Flags: review?(jdemooij) → review+

Comment 5

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/89c05305c708
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/0d05c148ef07 for apparently making hazard builds fail:
https://treeherder.mozilla.org/logviewer.html#?job_id=9810759&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=9811039&repo=mozilla-inbound
Flags: needinfo?(bhackett1024)

Comment 7

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c112db453761
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=9874995&repo=mozilla-inbound

Comment 9

3 years ago
Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4609a600f02

Comment 10

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/356231081116
Bouncy!

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/df00fd242b33 for (with retriggers) 2 of 10 Linux32 opt e10s browser-chrome runs crashing like https://treeherder.mozilla.org/logviewer.html#?job_id=9908263&repo=mozilla-inbound

Comment 12

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/322487136b28
https://hg.mozilla.org/mozilla-central/rev/322487136b28
Assignee: nobody → bhackett1024
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
The regression detection was quite pleased with this landing.
Overall this is an improvement on octane/kraken/ss.
There are some specific tests that regressed and might be interesting to look at,
but given the overall improvement, I'm not tracking the specific tests.

Mac OS X 10.10 32-bit (Mac Pro, shell)
- misc: typedobj-simple-struct-typedobj: 31.37% (regression)
- misc: typedobj-splay-typedobj: 2.66% (regression)
- misc: bugs-1131099-lodash2: 7.35% (regression)
- misc: basic-hoist-bounds-check: 2.84% (regression)
- octane: Box2D: -1.94% (regression)
- octane: Typescript: -4.39% (regression)
- dart: Richards: 17.91% (regression)

More details: http://arewefastyet.com/regressions/#/regression/1578297

Mac OS X 10.10 64-bit (Mac Pro, shell)
- kraken: stringify-tinderbox: 9.38% (regression)
- kraken: crypto-aes: 1.58% (regression)
- misc: bugs-847389-jpeg2000: 0.58% (regression)
- dart: Richards: 7.01% (regression)

More details: http://arewefastyet.com/regressions/#/regression/1578649
(Assignee)

Updated

3 years ago
Flags: needinfo?(bhackett1024)
Depends on: 1166730
Depends on: 1166542
status-firefox40: affected → ---
Depends on: 1166277
No longer depends on: 1166542
No longer depends on: 1166730
Depends on: 1168667
Depends on: 1167860

Updated

3 years ago
Depends on: 1171405
Depends on: 1182865
Depends on: 1182866
No longer depends on: 1182866

Updated

2 years ago
Depends on: 1189137
Depends on: 1216130
You need to log in before you can comment on or make changes to this bug.