Bug 70805 - [EFL] Make cache flush when max size of cache has been changed
Summary: [EFL] Make cache flush when max size of cache has been changed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-25 01:13 PDT by Tomasz Morawski
Modified: 2011-10-31 15:31 PDT (History)
6 users (show)

See Also:


Attachments
Make cache flush when max size of cache has been changed (1.52 KB, patch)
2011-10-25 01:15 PDT, Tomasz Morawski
no flags Details | Formatted Diff | Diff
Changlog file updated (1.70 KB, patch)
2011-10-25 06:28 PDT, Tomasz Morawski
no flags Details | Formatted Diff | Diff
Updated: changed oldSize to oldMax (1.70 KB, patch)
2011-10-31 03:53 PDT, Tomasz Morawski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomasz Morawski 2011-10-25 01:13:27 PDT
Calls the ewk_tile_unused_cache_auto_flush function after changing max size of cache when ewk_tile_unused_cache_max_set is called.
Comment 1 Tomasz Morawski 2011-10-25 01:15:08 PDT
Created attachment 112304 [details]
Make cache flush when max size of cache has been changed
Comment 2 Raphael Kubo da Costa (:rakuco) 2011-10-25 05:02:33 PDT
Looks OK, but it'd be good to explain what problems are being fixed by the patch.
Comment 3 Tomasz Morawski 2011-10-25 05:17:59 PDT
(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.
Comment 4 Raphael Kubo da Costa (:rakuco) 2011-10-25 05:28:56 PDT
I see. Could you add this explanation to the ChangeLog? Informal r+ from my side after that.
Comment 5 Tomasz Morawski 2011-10-25 06:28:53 PDT
Created attachment 112329 [details]
Changlog file updated
Comment 6 Ryosuke Niwa 2011-10-27 23:30:40 PDT
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.
Comment 7 Tomasz Morawski 2011-10-27 23:56:48 PDT
(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.
Comment 8 Ryosuke Niwa 2011-10-31 01:32:20 PDT
(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.
Comment 9 Tomasz Morawski 2011-10-31 03:53:00 PDT
Created attachment 113031 [details]
Updated: changed oldSize to oldMax

Updated: changed oldSize to oldMax
Comment 10 WebKit Review Bot 2011-10-31 15:31:31 PDT
Comment on attachment 113031 [details]
Updated: changed oldSize to oldMax

Clearing flags on attachment: 113031

Committed r98890: <http://trac.webkit.org/changeset/98890>
Comment 11 WebKit Review Bot 2011-10-31 15:31:36 PDT
All reviewed patches have been landed.  Closing bug.