Closed
Bug 1168864
Opened 7 years ago
Closed 7 years ago
Use mayResolve hook for addprop stubs
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
Details
Attachments
(2 files)
7.85 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
1.21 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
Websites/frameworks often add new properties to functions. Currently we can't inline the addprop operation because JSFunction has a resolve hook. This patch makes us check for the mayResolve hook I added in bug 1155946, so that we can inline property adds on functions and other objects with resolve hooks. With this patch + the patch in bug 935932, the micro-benchmark below improves from 1190 ms to 30 ms. function f() { for (var i=0; i<1000000; i++) { var res = function() {}; res.x = 2; res.y = 3; res.z = 4; } return res; } var t = new Date; f(); print(new Date - t);
Attachment #8611229 -
Flags: review?(bhackett1024)
![]() |
||
Comment 1•7 years ago
|
||
In CheckHasNoSuchProperty, shouldn't that be using curObj instead of obj?
Flags: needinfo?(jdemooij)
Comment 2•7 years ago
|
||
Comment on attachment 8611229 [details] [diff] [review] Patch Review of attachment 8611229 [details] [diff] [review]: ----------------------------------------------------------------- r+ with a fix for the bug bz found.
Attachment #8611229 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Not doing reviews right now from comment #1) > In CheckHasNoSuchProperty, shouldn't that be using curObj instead of obj? Good catch, thanks! Not sure how I missed that.
Flags: needinfo?(jdemooij)
Comment 5•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/db3d40e8b4de
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 6•7 years ago
|
||
Could this have caused and do we want to act on that? A regression/improvement was reported on AWFY: - slave: Mac OS X 10.10 32-bit (Mac Pro, shell) - mode: Ion Regression(s)/Improvement(s): - ss: raytrace: 4% (regression) Recorded range: - http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b563fdf07f83&tochange=20458c713464 More details: http://arewefastyet.com/regressions/#/regression/1689307
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 7•7 years ago
|
||
I added some checks for addProperty hooks, but we actually don't need to check for that one in CheckHasNoSuchProperty because it's only used for x.missingProp or "x in y". I thought addProperty hooks were uncommon but the ArrayObject class has one.
Flags: needinfo?(jdemooij)
Attachment #8615291 -
Flags: review?(bhackett1024)
Assignee | ||
Updated•7 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•7 years ago
|
Attachment #8615291 -
Flags: review?(bhackett1024) → review+
![]() |
||
Comment 8•7 years ago
|
||
> I thought addProperty hooks were uncommon
Fwiw, most DOM things have them. :(
Comment 10•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/306758f7b340
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•