[webkit-reviews] review granted: [Bug 22747] unbounded growth in the cache due to resource overhead : [Attachment 25939] new patch that calculates some sizes

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Stephanie Lewis
<slewis at apple.com>'s request for review:
Bug 22747: unbounded growth in the cache due to resource overhead
https://bugs.webkit.org/show_bug.cgi?id=22747

Attachment 25939: new patch that calculates some sizes
https://bugs.webkit.org/attachment.cgi?id=25939&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
> -    int delta = resource->size();
> +    int delta = resource->size() + resource->overheadSize();

I think it would be better for size() to automatically add in overheadSize(),
just like it adds in decodedSize(). Then, all these client calls can stay the
same.

> +    // The object must now be moved to a different queue, since its size may
have changed.
> +    // We have to remove explicitly before updating the size, so that we
find the correct previous
> +    // queue.
> +    int delta = sizeof(CachedResourceClient*)*m_clients.capacity();

A bunch of sites like this one need a space around the "*".

Because the overhead calculation is an estimate anyway, and it relies on a
bunch of assumptions, like the fact that we keep around a ResourceRequest for
every resource, and two copies, and the average size of ResourceRequest, this
seems like a lot of complexity just to come up with a paradoxically exact
estimate. I think it would be a lot simpler, and still solve our bug, if we
went with your original approach of a 4K estimate across the board.