Closed
Bug 928562
Opened 11 years ago
Closed 11 years ago
Remove uses of JSContext in IonBuilder for BytecodeAnalysis, BaselineInspector and bytecode types
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
Details
Attachments
(3 files)
27.90 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
1.02 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
1.01 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
BytecodeAnalysis and BaselineInspector have no real need for a JSContext. The bytecode type map in script->types is a little bit trickier, as it is not guaranteed to already exist. The basic problem is its lifetime is very slightly different from that of the baseline script --- when a GC occurs with the baseline script on the stack, script->types->bytecodeMap will be cleared but script->baselineScript will not be. Rather than add more complexity to deal with this, this patch just moves the bytecode map to the baseline script.
Attachment #819235 -
Flags: review?(jdemooij)
Updated•11 years ago
|
Attachment #819235 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 1•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/42e53d0401e5
Comment 2•11 years ago
|
||
This patch regressed Deltablue and Raytrace on AWFY.
Comment 3•11 years ago
|
||
(But improved Kraken Astar and Oscillator as you were expecting)
Assignee | ||
Comment 4•11 years ago
|
||
Oops, since we now need to baseline compile scripts before running IonBuilder on them we should ensure that scripts we try to inline during the definite properties analysis are baseline compiled first.
Assignee: nobody → bhackett1024
Attachment #820438 -
Flags: review?(jdemooij)
Comment 5•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/42e53d0401e5
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment 6•11 years ago
|
||
This patch seems to have regressed V8v7 on inbound by 8% or so... is that expected?
Flags: needinfo?(bhackett1024)
Updated•11 years ago
|
Attachment #820438 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to On vacation Oct 12 - Oct 27 from comment #6) > This patch seems to have regressed V8v7 on inbound by 8% or so... is that > expected? This patch should fix the regression, see comments 2 and 4. https://hg.mozilla.org/integration/mozilla-inbound/rev/89b5b123e01a
Flags: needinfo?(bhackett1024)
Comment 8•11 years ago
|
||
It fixed the Raytrace regression, but not the Deltablue one.
Comment 9•11 years ago
|
||
(In reply to Guilherme Lima from comment #8) > It fixed the Raytrace regression, but not the Deltablue one. Setting needinfo.
Flags: needinfo?(bhackett1024)
Assignee | ||
Comment 12•11 years ago
|
||
The followup fix was not quite right, as it would only eagerly baseline compile scripts if they had never executed at all.
Attachment #822471 -
Flags: review?(jdemooij)
Flags: needinfo?(bhackett1024)
Updated•11 years ago
|
Attachment #822471 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cee39f8dc049
Comment 14•11 years ago
|
||
(In reply to Brian Hackett (:bhackett) from comment #13) > https://hg.mozilla.org/integration/mozilla-inbound/rev/cee39f8dc049 Thanks!
Comment 15•11 years ago
|
||
And this new version fixed Deltablue but regressed Raytrace. lol
Comment 17•11 years ago
|
||
(In reply to Guilherme Lima from comment #15) > And this new version fixed Deltablue but regressed Raytrace. lol Is there another bug for this, and if not, should there be?
You need to log in
before you can comment on or make changes to this bug.
Description
•