Bug 70805 – [EFL] Make cache flush when max size of cache has been changed

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Tomasz Morawski]

Calls the ewk_tile_unused_cache_auto_flush function after changing max size of cache when ewk_tile_unused_cache_max_set is called.

[Tomasz Morawski]

Created attachment 112304 [details]
Make cache flush when max size of cache has been changed

[Raphael Kubo da Costa (:rakuco)]

Looks OK, but it'd be good to explain what problems are being fixed by the patch.

[Tomasz Morawski]

(In reply to comment #2)
> Looks OK, but it'd be good to explain what problems are being fixed by the patch.

OK. Which out of this patch when ewk_tile_unused_cache_max_set was called and 
max set is lower that old one value. The cache may use more memory than set max
value and this is not expected by user of this class.

[Raphael Kubo da Costa (:rakuco)]

I see. Could you add this explanation to the ChangeLog? Informal r+ from my side after that.

[Tomasz Morawski]

Created attachment 112329 [details]
Changlog file updated

[Ryosuke Niwa]

Comment on attachment 112329 [details]
Changlog file updated

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

I'm confused. How does this help? ewk_tile_unused_cache_auto_flush has an early exit:
if (tileUnusedCache->memory.used <= tileUnusedCache->memory.max)
    return;
so it seems like calling ewk_tile_unused_cache_auto_flush here does't do anything.

> Source/WebKit/efl/ewk/ewk_tiled_model.cpp:636
> +    size_t oldSize = tileUnusedCache->memory.max;

I'd prefer calling it oldMax instead.

[Tomasz Morawski]

(In reply to comment #6)
For example we have a cache that currently store 20MB of data
* the tileUnusedCache->memory.used = 20MB
and max value is set 40MB 
tileUnusedCache->memory.max= 40MB

Now, a user calls:
ewk_tile_unused_cache_max_set(tuc, 10MB)
the cache after calling of this function should use only 10MB. Due to that 
I call ewk_tile_unused_cache_auto_flush to release some memory.

The value of used and max will be:
> ewk_tile_unused_cache_auto_flush has an early exit:
if (tileUnusedCache->memory.used(20MB) <= tileUnusedCache->memory.max(10MB))
    return;

the "if" condition is false in that situation, so the early exit it not occured in this case and the memory form cache will be released.

[Ryosuke Niwa]

(In reply to comment #7)
> (In reply to comment #6)
> For example we have a cache that currently store 20MB of data
> * the tileUnusedCache->memory.used = 20MB
> and max value is set 40MB 
> tileUnusedCache->memory.max= 40MB
> 
> Now, a user calls:
> ewk_tile_unused_cache_max_set(tuc, 10MB)
> the cache after calling of this function should use only 10MB. Due to that 
> I call ewk_tile_unused_cache_auto_flush to release some memory.
> 
> The value of used and max will be:
> > ewk_tile_unused_cache_auto_flush has an early exit:
> if (tileUnusedCache->memory.used(20MB) <= tileUnusedCache->memory.max(10MB))
>     return;
> 
> the "if" condition is false in that situation, so the early exit it not occured in this case and the memory form cache will be released.

Makes sense. But I'd still prefer calling that variable oldMax than oldSize unless it's really the old size.

[Tomasz Morawski]

Created attachment 113031 [details]
Updated: changed oldSize to oldMax

Updated: changed oldSize to oldMax

[WebKit Review Bot]

Comment on attachment 113031 [details]
Updated: changed oldSize to oldMax

Clearing flags on attachment: 113031

Committed r98890: <http://trac.webkit.org/changeset/98890>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.