Bug 110560 – Code cache size should adapt to workload

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

Code cache size should adapt to workload

[Geoffrey Garen]

Created attachment 189699 [details]
Patch

[Geoffrey Garen]

Created attachment 189700 [details]
microbenchmark

[Antti Koivisto]

Comment on attachment 189699 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=189699&action=review

r=me

> Source/JavaScriptCore/ChangeLog:11
> +
> +        (*) 5% PLT arithmetic mean speedup
> +        (*) 10% PLT geometric mean speedup
> +        (*) 3.4X microbenchmark speedup
> +        (*) Reduces initial cache capacity by 16X

Nice!

How does this affect average cache capacity (say over PLT3)?

> Source/JavaScriptCore/runtime/CodeCache.h:156
> +        if (age > m_capacity) {
> +            // A requested object is older than the cache's capacity. We can
> +            // infer that requested objects are subject to high eviction probability,
> +            // so we grow the cache to improve our hit rate.
> +            m_capacity += s_recencyBias * s_oldObjectSamplingMultiplier * key.length();

Is it plausible that some access pattern (say XHR of lots of unique scripts with occasional hits to aging cache items) could make cache grow very large without providing good hit rate? Should there be MaxCacheCapacity?

> Source/JavaScriptCore/runtime/CodeCache.h:190
> +    static const int64_t s_recencyBias = 4;

I would name constants without s_

[Geoffrey Garen]

> How does this affect average cache capacity (say over PLT3)?

On PLT3, cache capacity was 16MC, and now it grows to about 25MC. PLT has a 99% cache hit rate, so I think this is reasonable.

On an site like nytimes.com, cache capacity can be around 4MC - 8MC. Navigating some nytimes.com pages, followed by wsj.com pages, followed by amazon.com pages, in a cycle, puts the cache at about 16MC. Hit rates vary from about 50%-99%, depending on the site.

Running the attached microbenchmark, you get up to about 64MC. Critically, navigating nytimes.com after running the microbenchmark, the cache shrinks back down to 4MC - 8MC after about 10-20 page loads.

* MC: million characters.

Side note: Rough measurement indicates that cached bytecode is much more compact than original text. So, it's a big optimization that the cache no longer holds onto the string key. For any resource whose text is already in the WebCore cache, adding a bytecode cache for it seems like small additional overhead with big speed gain.

> > Source/JavaScriptCore/runtime/CodeCache.h:156
> > +        if (age > m_capacity) {
> > +            // A requested object is older than the cache's capacity. We can
> > +            // infer that requested objects are subject to high eviction probability,
> > +            // so we grow the cache to improve our hit rate.
> > +            m_capacity += s_recencyBias * s_oldObjectSamplingMultiplier * key.length();
> 
> Is it plausible that some access pattern (say XHR of lots of unique scripts with occasional hits to aging cache items) could make cache grow very large without providing good hit rate? Should there be MaxCacheCapacity?

Seems unlikely, but plausible. (The key factor that makes this unlikely is that pulling an item out of the cache updates its age. The only pattern that can trigger the bad case is a repeating cycle of 1 item, followed by hundreds of unique items, followed by that first item again. And, even though this pattern will make a cache much bigger than it needs to be, it will still have bounded space.)

I think it's worth investigating a max and, separately, investigating evicting unrepeated items first. I think I'll do that in a follow-up patch, since qualifying this patch took lots and lots of performance measurement time.

> > Source/JavaScriptCore/runtime/CodeCache.h:190
> > +    static const int64_t s_recencyBias = 4;
> 
> I would name constants without s_

Will fix. I've always wondered what our style guideline was for that.

Thanks!

[Geoffrey Garen]

Committed r143759: <http://trac.webkit.org/changeset/143759>