1116349 - Flush Picasso LRU cache on memory pressure events

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Comment 1

[Richard Newman [:rnewman]]

Google should be sufficient to figure out how to do this.

The right place to add the code is probably MemoryMonitor.java.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://mail.mozilla.org/pipermail/mobile-firefox-dev/2015-February/001090.html

Comment 3

[Melchiorre Alastra (:Mailkov)]

I'd like to work on this ... can you assign to me ?

Comment 4

[Melchiorre Alastra (:Mailkov)]

Attachment: bug1116349.patch

Comment 5

[Melchiorre Alastra (:Mailkov)]

I create a patch ... I hope it is ok ... :)

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8634747 [details] [diff] [review]
bug1116349.patch

Review of attachment 8634747 [details] [diff] [review]:
-----------------------------------------------------------------

Eviction code seems correct, but I'm not sure under which memory pressure level we should be flushing Picasso - deferring to Richard.

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 8634747 [details] [diff] [review]
bug1116349.patch

Review of attachment 8634747 [details] [diff] [review]:
-----------------------------------------------------------------

This fires at MEDIUM and above. MEDIUM corresponds to:


    /**
     * Level for {@link #onTrimMemory(int)}: the process is not an expendable
     * background process, but the device is running moderately low on memory.
     * Your running process may want to release some unneeded resources for
     * use elsewhere.
     */
    static final int TRIM_MEMORY_RUNNING_MODERATE = 5;


That seems right to me. Worst-case, Picasso can go back to disk.

::: mobile/android/base/home/ImageLoader.java
@@ +47,5 @@
>          }
>      }
>  
>      private static Picasso instance;
> +    private static LruCache lrucache;

Add a comment that these are protected by synchronization on the class.

@@ +62,5 @@
>  
>          return instance;
>      }
>  
> +    public static void clearLruCache() {

This method should be synchronized.

Comment 8

[Melchiorre Alastra (:Mailkov)]

Attachment: Bug 1116349 - Flush Picasso LRU cache on memory pressure events

I synchronized the function clearLruCache() and add comment as you suggested.

I think now the patch is ok ... If it is, please can you push my patch to Try server? Because I have not yet access. 
Thanks.

Comment 9

[Richard Newman [:rnewman]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b2507071cc8

Comment 10

[Richard Newman [:rnewman]]

Leaving ni for me to check the try run and land.

Comment 11

[Richard Newman [:rnewman]]

url:        https://hg.mozilla.org/integration/fx-team/rev/9b6528285beac04e2a29cc6ad6220067f6930604
changeset:  9b6528285beac04e2a29cc6ad6220067f6930604
user:       Melchiorre Alastra <[e-mail]>
date:       Wed Jul 22 18:00:07 2015 -0700
description:
Bug 1116349 - Flush Picasso LRU cache on memory pressure events

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9b6528285bea