Closed Bug 928562 Opened 6 years ago Closed 6 years ago

Remove uses of JSContext in IonBuilder for BytecodeAnalysis, BaselineInspector and bytecode types

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: bhackett, Assigned: bhackett)

References

Details

Attachments

(3 files)

Attached patch patchSplinter 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)
Blocks: 928464
Attachment #819235 - Flags: review?(jdemooij) → review+
This patch regressed Deltablue and Raytrace on AWFY.
(But improved Kraken Astar and Oscillator as you were expecting)
Attached patch patchSplinter Review
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)
https://hg.mozilla.org/mozilla-central/rev/42e53d0401e5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
This patch seems to have regressed V8v7 on inbound by 8% or so... is that expected?
Flags: needinfo?(bhackett1024)
Attachment #820438 - Flags: review?(jdemooij) → review+
(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)
It fixed the Raytrace regression, but not the Deltablue one.
Blocks: 927383
(In reply to Guilherme Lima from comment #8)
> It fixed the Raytrace regression, but not the Deltablue one.

Setting needinfo.
Flags: needinfo?(bhackett1024)
Duplicate of this bug: 928464
Attached patch patchSplinter Review
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)
Attachment #822471 - Flags: review?(jdemooij) → review+
And this new version fixed Deltablue but regressed Raytrace. lol
Depends on: 931496
(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.