[RFC][Icedtea-Web]: Enforce cache size limit.

[Deepak Bhole]

* Andrew Su <asu at redhat.com> [2011-04-18 23:04]:
> Hello,
> 
> This patch will ensure the cache size will remain at most the size specified in the user's deployment's properties file.
> 
> This is executed only at jvm shutdown of the plugin and/or javaws.
> 
> Questions? Comments? Concerns?
> 
> Regards,
>   Andrew

> diff -r 211a5e73d119 netx/net/sourceforge/jnlp/cache/CacheUtil.java
> --- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Mon Apr 18 17:38:31 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Mon Apr 18 22:58:38 2011 -0400
> @@ -533,6 +533,15 @@
>              // First we want to figure out which stuff we need to delete.
>              HashSet<String> keep = new HashSet<String>();
>              lruHandler.load();
> +            
> +            long maxSize = -1; // Default 
> +            try {
> +                maxSize = Long.parseLong(JNLPRuntime.getConfiguration().getProperty("deployment.cache.max.size"));
> +            } catch (NumberFormatException nfe) {
> +            }
> +            
> +            maxSize = maxSize << 20; // Convert to byte (Should not overflow..)

Please clarify the above comment along the lines of "Convert from
megabytes to bytes" or something, as it is not immediately clear.

> +            long curSize = 0;
>  
>              for (Entry<String, String> e : lruHandler.getLRUSortedEntries()) {
>                  // Check if the item is contained in cacheOrder.
> @@ -540,16 +549,19 @@
>                  final String value = e.getValue();
>  
>                  if (value != null) {
> +                    File file = new File(value);
>                      PropertiesFile pf = new PropertiesFile(new File(value + ".info"));
>                      boolean delete = Boolean.parseBoolean(pf.getProperty("delete"));
>  
>                      // This will get me the root directory specific to this cache item.
> -                    String rStr = value.substring(cacheDir.length());
> +                    String rStr = file.getPath().substring(cacheDir.length());
>                      rStr = cacheDir + rStr.substring(0, rStr.indexOf(File.separatorChar, 1));

This is rather convoluted. Can you add an example in the comments
stating exactly what each of the above lines do to a sample "value"?

>From what I can tell, it is just getting the dirname of the file. If
that is correct, why not use File.getParentFile()?


> +                    long len = file.length();
>  
> -                    if (delete || keep.contains(rStr)) {
> +                    if (delete || !file.isFile() || (maxSize >= 0 && curSize + len > maxSize) || keep.contains(rStr)) {

Can you clarify the logic here (in a comment as well) please?

1. if delete  == true, delete .... this is OK

2. if !file.isFile() .... would getLRUSortedEntries() ever return a dir?

3. (maxSize >= 0 && curSize + len > maxSize) ... okay, sounds
   reasonable, this is current behaviour

4. keep.contains(rStr) ... the else part of that block adds rStr to
   keep. Can there be a case where it shows up again during the iteration?

Cheers,
Deepak

>                          lruHandler.removeEntry(key);
>                      } else {
> +                        curSize += len;
>                          keep.add(value.substring(rStr.length()));
>                          keep.add(rStr); // We can just use the same map, since these two things are disjoint with each other.
>                      }