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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 17 13:04:23 PDT 2016


            Bug ID: 163559
           Summary: calculateMemoryCacheSizes incorrectly pins
    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>

More information about the webkit-unassigned mailing list