Bug 110560 - Code cache size should adapt to workload
Summary: Code cache size should adapt to workload
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-21 23:35 PST by Geoffrey Garen
Modified: 2013-02-22 11:17 PST (History)
3 users (show)

See Also:


Attachments
Patch (5.64 KB, patch)
2013-02-21 23:40 PST, Geoffrey Garen
koivisto: review+
Details | Formatted Diff | Diff
microbenchmark (681 bytes, text/html)
2013-02-21 23:43 PST, Geoffrey Garen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2013-02-21 23:35:41 PST
Code cache size should adapt to workload
Comment 1 Geoffrey Garen 2013-02-21 23:40:04 PST
Created attachment 189699 [details]
Patch
Comment 2 Geoffrey Garen 2013-02-21 23:43:10 PST
Created attachment 189700 [details]
microbenchmark
Comment 3 Antti Koivisto 2013-02-22 07:24:33 PST
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_
Comment 4 Geoffrey Garen 2013-02-22 11:14:01 PST
> 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!
Comment 5 Geoffrey Garen 2013-02-22 11:17:40 PST
Committed r143759: <http://trac.webkit.org/changeset/143759>