Bug 200159 – Make NetworkCache cache shrink logic take account of the configured capacity

• Component: Page Loading
• Status: NEW

[Loïc Yhuel]

Make NetworkCache cache shrink logic take account of the configured capacity

[Loïc Yhuel]

Created attachment 374958 [details]
Patch

[Loïc Yhuel]

I'm open to comments about the logic, since :
 - I had to remove the "effectiveWorth" adjustment : keeping it would prevent an unspecified amount of records to be deleted, and I don't know how we could reimplement something similar.
 - The computed deletion probability can be greater than 1 for a part of the entries (if the cache is much bigger than the capacity, either due to issues or if we are reducing it, or if the average worth is high), which means the deletionProbabilityFactor could in fact be not big enough to delete entries with high worth.
 - If the average worth is high, the shrink can also be too aggressive.

[Loïc Yhuel]

Created attachment 374959 [details]
Patch

[Chris Dumez]

Comment on attachment 374959 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374959&action=review

> Source/WebKit/ChangeLog:9
> +        more than 33%, we should target 67% of the capacity instead.

Why? 

This change log does not explain why this change is needed.

[Loïc Yhuel]

(In reply to Chris Dumez from comment #4)
> > Source/WebKit/ChangeLog:9
> > +        more than 33%, we should target 67% of the capacity instead.
> 
> Why? 
> 
> This change log does not explain why this change is needed.
It's "when reducing the cache capacity". The target shrink size must depend on the desired capacity, not the current size.

For example if you have a 200MB cache (capacity >= 200MB), and you call WebKit::NetworkCache::Cache::setCapacity(100MB), the original code has a maximum deletion probability of 33% so to cache would be reduced to 134MB at best, so you would need at laest a second shrink to be under 100MB.