Closed Bug 876434 Opened 7 years ago Closed 7 years ago

BaselineCompiler: Compile JSOP_CALLEE

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox23 --- fixed
firefox24 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
Ion already compiles it (not currently though because baseline aborts) and JSOP_CALLEE isn't that uncommon.
Attachment #754474 - Flags: review?(kvijayan)
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 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?
(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?
I think we should consider uplifting after it has been in a nightly or two.

https://hg.mozilla.org/integration/mozilla-inbound/rev/61ec49c34885
(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.
https://hg.mozilla.org/mozilla-central/rev/61ec49c34885
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(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 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.
Thanks for the ping Lukas.  Aurora approval form filled out.
Flags: needinfo?(kvijayan)
Attachment #754474 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.