1858746 - atomize more nursery strings during deduplication

Status: ASSIGNED

(Core :: JavaScript Engine, task, P3)

Description

[Steve Fink [:sfink] [:s:]]

While writing a test for my stats gathering for bug 1846297, I noticed that my test string that I was using as a lookup key was not getting atomized during deduplication as I expected. I discovered that atomization during deduplication only happens for longer strings (StringToAtomCache::MinStringLength, 39 chars), but that threshold appears to have been chosen because shorter strings are faster to hash and so the cost of an extra cache lookup is less worth it. That logic doesn't apply as much to deduplication.

So I experimented with allowing longer than inline but shorter than StringToAtomCache::MinStringLength strings to be deduplicated. It's more code duplication and change than I'd like, and the number of strings this applies to is surprisingly (to me) small.

Speedometer 3 without the patch:

               ReplaceWithAtom count=      457   0.0% bytes=       31964   0.1%
                   Deduplicate count=   876726  42.8% bytes=     8960350  30.7%
           CopyInlineToTenured count=   729033  35.6% bytes=     7245160  24.8%
             CopyRopeToTenured count=   188288   9.2% bytes=           0   0.0%
        CopyDependentToTenured count=   169384   8.3% bytes=           0   0.0%
         CopyMallocedToTenured count=    83481   4.1% bytes=    12925836  44.3%

With the patch, so that mid-size strings that have been atomized will be atomized during tenuring:

               ReplaceWithAtom count=     3013   0.1% bytes=      106786   0.4%
                   Deduplicate count=   855339  42.2% bytes=     8829713  32.3%
           CopyInlineToTenured count=   744519  36.7% bytes=     7369113  26.9%
             CopyRopeToTenured count=   177470   8.7% bytes=           0   0.0%
        CopyDependentToTenured count=   169135   8.3% bytes=           0   0.0%
         CopyMallocedToTenured count=    79735   3.9% bytes=    11053080  40.4%

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1858746 - Set IN_STRING_TO_ATOM_CACHE for short strings and deduplicate to atoms during minor GC.

Comment 2

[Steve Fink [:sfink] [:s:]]

(Note that the byte counts for ropes and dependent strings are zero because I'm only counting the owned chars, since otherwise ropes will count their constituent chars many, many times and the percentages will get washed out by the quadratic overcounting.)

Comment 3

[Steve Fink [:sfink] [:s:]]

We sort of have a table of different buckets of strings, and it might be useful to change the behavior for some of the buckets. The buckets are defined by their length (so, short inline vs longer inline vs short out of line vs longer out of line, where the exact lengths of the bucket divisions would need to be guessed or tuned.) And also whether the string has ever been looked up as an atom, and/or is currently in the string to atom cache (which is a superset of the former).

I didn't paste it in, but the string stats I pasted above also have a histogram of string lengths for strings that get tenured. Just knowing the length distribution doesn't really say what the best behavior would be, though.