Closed
Bug 876434
Opened 8 years ago
Closed 8 years ago
BaselineCompiler: Compile JSOP_CALLEE
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(1 file)
2.02 KB,
patch
|
djvj
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Ion already compiles it (not currently though because baseline aborts) and JSOP_CALLEE isn't that uncommon.
Attachment #754474 -
Flags: review?(kvijayan)
Comment 1•8 years ago
|
||
Seems like we would never ion compile JSOP_CALLEE in 23. I think we had some performance fault with that in the past, maybe we should consider uplifting to aurora.
Comment 2•8 years ago
|
||
Comment on attachment 754474 [details] [diff] [review] Patch Review of attachment 754474 [details] [diff] [review]: ----------------------------------------------------------------- Good catch. This would have prevented ion compilation of CALLEE functions when baseline is enabled. I agree with Tom, we should see if we can uplift into 23.
Attachment #754474 -
Flags: review?(kvijayan)
Attachment #754474 -
Flags: review+
Attachment #754474 -
Flags: approval-mozilla-aurora?
Comment 3•8 years ago
|
||
(In reply to Kannan Vijayan [:djvj] from comment #2) > I agree with Tom, we should see if we can uplift into 23. It would help us to evaluate an uplift request if you filled out the form that comes up when you set the ? - mainly what is the risk involved here and what is the user impact if this is not uplifted?
Assignee | ||
Comment 4•8 years ago
|
||
I think we should consider uplifting after it has been in a nightly or two. https://hg.mozilla.org/integration/mozilla-inbound/rev/61ec49c34885
Comment 5•8 years ago
|
||
(In reply to lsblakk@mozilla.com [:lsblakk] from comment #3) > (In reply to Kannan Vijayan [:djvj] from comment #2) > > > I agree with Tom, we should see if we can uplift into 23. > > It would help us to evaluate an uplift request if you filled out the form > that comes up when you set the ? - mainly what is the risk involved here and > what is the user impact if this is not uplifted? Sorry, will do once it's gone through a couple of nightlies as Jan suggests.
Comment 6•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/61ec49c34885
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Comment 7•8 years ago
|
||
(In reply to Kannan Vijayan [:djvj] from comment #5) > > Sorry, will do once it's gone through a couple of nightlies as Jan suggests. Now that it has been in some nightlies - any new info to share?
Flags: needinfo?(kvijayan)
Comment 8•8 years ago
|
||
Comment on attachment 754474 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): N/A User impact if declined: Low performance. Any function which uses JSOP_CALLEE (e.g. recursive functions) will not be ion-compiled. Testing completed (on m-c, etc.): Made it through a couple of nightlies without any major issues reported. Risk to taking this patch (and alternatives if risky): Very low. Baseline codegeneration for this is trivial, and the Ion version already exist and has been tested. String or IDL/UUID changes made by this patch: None.
Comment 9•8 years ago
|
||
Thanks for the ping Lukas. Aurora approval form filled out.
Flags: needinfo?(kvijayan)
Updated•8 years ago
|
Attachment #754474 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/8b5e5b168e78
status-firefox23:
--- → fixed
status-firefox24:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•