Closed Bug 1522186 Opened 6 years ago Closed 4 years ago

Stop pretenuring strings when the tenuring rate drops

Categories

(Core :: JavaScript: GC, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
firefox85 --- fixed

People

(Reporter: sfink, Assigned: allstars.chh)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Currently, a minor GC will look at how many strings have been tenured, and stop allocating strings in the nursery for any zone that tenures too many.

This is a one-way switch. Once you start pretenuring, you will always pretenure (for that Zone). So if you have a workload that creates a bunch of long-lived strings during its initialization phase, but then later tends to generate short-lived strings, you could end up stuck pretenuring.

I wrote a patch that looks at how many strings are collected during a major GC. If there are too many, then it switches string nursery allocation back on. In my testing, I wasn't able to tune it to prevent it from slowing down Octane. But it would be good functionality to have.

Octane deserves a further note: without the ability to turn off string pretenuring, Octane will run the first several tests with nursery strings, then switch them off, and the rest of the Octane subtests will run with nursery strings disabled. It would be good if it could find a good place to switch them back on.

I was only using the number of garbage strings during a major GC to decide when to do this. Perhaps it would also be useful to consider that if you do a nursery collection (or several) where you don't tenure very many strings, then it might be a good time to turn it back on? At the very least, it shouldn't hurt all that much to turn it on; if you're not allocating very many strings in the first place, then where they're allocated doesn't matter much.

Priority: -- → P3
Assignee: nobody → allstars.chh

Hi, Jonco
I try to count the numbers of tenured allocated strings and marked strings in the zone, in order to calculate the survival rate of strings.
Can you check if I do this correctly?

Thanks

Flags: needinfo?(jcoppeard)

(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #2)
The patch looks OK (I commented) but the question is how we can use this information effectively.

Flags: needinfo?(jcoppeard)

Updated my patch to have a more accurate number.

I try to use cellIter<JSString> and cellIter<JSFatInlineString> to count the number of strings in the zone when GC is in js::gc::State::Finish, and in most cases the number is matched with my patch ( zone->tenuredAllocatedStrings - zone->tenuredFinalizedStrings).

But I found when the zone is close to do string pretenuring (mostly after we turn on string pretenuring, I found sometimes in the previous run of MajorGC will also have this problem), the number starts to mismatch, for example, using cellIter will count 1,329,000 strings, but my patch will count 1,339,000 strings), with about 10k difference. And the number is going to bigger and bigger after running.

However, I am not sure if I can use zone->cellIter<JSString>() to count the number of JSStrings in the zone?
As in the ZoneAllCellIter<TenuredCell>::init it asserts either it's an atom zone or the nursery should be empty, which is neither true in my case
https://searchfox.org/mozilla-central/rev/c938c7416c633639a5c8ce4412be586eefb48005/js/src/gc/GC-inl.h#129

Jonco, could you give me some suggestions?

Thanks

Flags: needinfo?(jcoppeard)

Oh, found the problem, I increments the counter directly without checking if allocation succeeds.

Flags: needinfo?(jcoppeard)

I try some benchmarks.
JetStream2 and octane will have the tenured strings marked rate around 30% ~ 90%.
ARES-6 and Speedometer doesn't have too much on string pretenuring.

The interesting one ins v8's web tooling benchmark,
https://github.com/v8/web-tooling-benchmark
It will have cases that the marked rate is like 17%, so if we decide to turn off string pretenuring, later in minor GC it will have many strings got tenured again so we turn on string pretenuing. The situation will last for a few times.

So I compare the results with a lower threhold 0.15 and a higher one 0.30

https://docs.google.com/spreadsheets/d/1sLHBu_QZ-xUs1CeHZlnX93Cezmxqvx3BTMs70BGhKog/edit?usp=sharing

Summary:
Although 0.15 threshold will spend more time on MajorGC (7% compare to 0.30 threshold, which is spending 1000ms more )
but it will save more time in MinorGC (25% less, which is 7,700,000 ms less)
And the result of the benchmark is 5% better than 0.30 theshold. (one subtest has 80% improvement)

Attachment #9182877 - Attachment description: Bug 1522186(WIP) - Keep track of tenuredAllocatedStrings and markedStrings in zone. → Bug 1522186 - Disable string pretenuring once the rate is low.
Attachment #9182877 - Attachment description: Bug 1522186 - Disable string pretenuring once the rate is low. → Bug 1522186 - Disable string pretenuring once the finalization rate is high.

Hi Jonco
I've updated the patch on your suggestion. However, I found this method will over-count some strings.

For example, if the arena still has some free space left before major GC, and later while during the major GC, some strings will be allocated on that arena.
https://searchfox.org/mozilla-central/rev/1843375acbbca68127713e402be222350ac99301/js/src/gc/Allocator.cpp#566

So when we count the nmarked during finalization phrase, the strings allocated during major GC will also be counted.
The overcount percentage is 1~2%
(for example, use cellIter to count the number of strings before the major GC will get 403398, and count the (nmarked + nfinalized) during FinalizedTypedArena will get 414172)

I could add some flags in those arenas, but as I remembered we have removed the flag allocatedDuringIncremental in arena, not sure if it's a good idea to add it back.

Can you give me some suggestions?

Thanks

Flags: needinfo?(jcoppeard)

(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #7)
This a heuristic and doesn't have to be exact. 2% probably doesn't matter.

Flags: needinfo?(jcoppeard)
Pushed by allstars.chh@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ce4363056f26 Disable string pretenuring once the finalization rate is high. r=jonco
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: