[Webkit-unassigned] [Bug 163559] New: calculateMemoryCacheSizes incorrectly pins cacheMaxDeadCapacity

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=163559

            Bug ID: 163559
           Summary: calculateMemoryCacheSizes incorrectly pins
                    cacheMaxDeadCapacity
    Classification: Unclassified
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: WebKit2
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: krollin at apple.com
                CC: akling at apple.com, cgarcia at igalia.com,
                    ggaren at apple.com, koivisto at iki.fi

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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161017/f1c2b0ea/attachment-0001.html>