Bug 163559 – calculateMemoryCacheSizes incorrectly pins cacheMaxDeadCapacity

• Status: NEW
• Component: WebKit2

[Keith Rollin]

WebKit2/Shared/CacheModel.cpp has the following:

        // Object cache capacities (in bytes)
        // (Testing indicates that value / MB depends heavily on content and
        // browsing pattern. Even growth above 128MB can have substantial
        // value / MB for some content / browsing patterns.)
        if (memorySize >= 2048)
            cacheTotalCapacity = 128 * MB;
        else if (memorySize >= 1536)
            cacheTotalCapacity = 96 * MB;
        else if (memorySize >= 1024)
            cacheTotalCapacity = 64 * MB;
        else if (memorySize >= 512)
            cacheTotalCapacity = 32 * MB;

        cacheMinDeadCapacity = cacheTotalCapacity / 4;
        cacheMaxDeadCapacity = cacheTotalCapacity / 2;

        // This code is here to avoid a PLT regression. We can remove it if we
        // can prove that the overall system gain would justify the regression.
        cacheMaxDeadCapacity = std::max(24u, cacheMaxDeadCapacity);

That last line looks like it should use 24 * MB rather than just 24u. Geoff Garen wrote the original code. He says:

> You’re right — that code is clearly wrong. I’d suggest deleting it.
> 
> Perhaps the code was right at one time but became wrong over time through refactoring.
> 
> Anyway, the PLT it speaks of is an old PLT we don’t run anymore, so we don’t need to worry about it.
> 
> It looks like this means that 512MB devices only get 16MB of dead object cache. That’s pretty small for today’s internet — only one or two pages. Still, it might be appropriate, since 512MB is pretty small.

This change should probably be made by someone who knows the ins-and-outs of testing the effect of the change, so cc'ing the last few people who touched this file in case they are interested in taking it.