Closed Bug 1271609 Opened 4 years ago Closed 4 years ago

ReleaseAllJITCode should call Zone::discardJitCode instead of duplicating it

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: jandem, Assigned: jandem)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
I'm working on a patch for this code, let's deduplicate it first.

 1 files changed, 2 insertions(+), 28 deletions(-)
Attachment #8750750 - Flags: review?(jcoppeard)
Comment on attachment 8750750 [details] [diff] [review]
Patch

Review of attachment 8750750 [details] [diff] [review]:
-----------------------------------------------------------------

Good idea.

::: js/src/jsgc.cpp
@@ +7141,5 @@
>      fop->runtime()->gc.evictNursery();
>  
>      for (ZonesIter zone(fop->runtime(), SkipAtoms); !zone.done(); zone.next()) {
> +        zone->setPreservingCode(false);
> +        zone->discardJitCode(fop);

Zone::discardJitCode also calls JSScript::resetWarmUpCounter, so this will now happen when JIT code is discarded outside of a GC.  Do you think that will be a problem?
Attachment #8750750 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #1)
> Zone::discardJitCode also calls JSScript::resetWarmUpCounter, so this will
> now happen when JIT code is discarded outside of a GC.  Do you think that
> will be a problem?

Hm I missed that somehow, thanks.

resetWarmUpCounter is actually what we want here - after we throw out all JIT code we should go through the normal interpreter -> Baseline -> Ion warmup cycle. Baseline collects information Ion depends on, so jumping straight into Ion doesn't work well.
https://hg.mozilla.org/mozilla-central/rev/0e4369105765
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.