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

[Andrew Su]

----- Original Message -----
> From: "Deepak Bhole" <dbhole at redhat.com>
> To: "Andrew Su" <asu at redhat.com>
> Cc: "IcedTea" <distro-pkg-dev at openjdk.java.net>
> Sent: Tuesday, April 19, 2011 1:22:12 AM
> Subject: Re: [RFC][Icedtea-Web]: Enforce cache size limit.
> * 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.

done

> 
> > + 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()?

When I said root directory, I meant it as in the unique folder to the item.
Such as "/home/user1/.icedtea/cache/3"

> 
> 
> > + 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?

No, and it shouldn't, but let's say, directory was created but the
actual item doesn't exist. If the item wasn't marked for deletion
we'll have an empty folder lying around, should get rid of it.
(This seems to go with previous patch more, missed this case)

> 
> 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?

Nope, it doesn't the second add to keep is for use with removeUntrackedDirectories()
Cheers,
  Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20110419_cache_limit_v2.patch
Type: text/x-patch
Size: 3419 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110419/422ea5ef/20110419_cache_limit_v2.patch