Bug 163559
Summary: | calculateMemoryCacheSizes incorrectly pins cacheMaxDeadCapacity | ||
---|---|---|---|
Product: | WebKit | Reporter: | Keith Rollin <krollin> |
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> |
Status: | NEW | ||
Severity: | Normal | CC: | cgarcia, ggaren, kling, koivisto |
Priority: | P2 | ||
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified |
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.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |