333202 - JAR cache not working?

Status: RESOLVED WORKSFORME

(Core :: Networking, defect)

Description

[Darin Fisher]

JAR cache not working?

I logged calls to CreateFile in NSPR, and I found that several JAR files are opened many times during startup.  However, our JAR files are supposed to only be opened once and cached for later use.

I'll attach my log of CreateFile calls shortly.

Comment 1

[Darin Fisher]

Attachment: CreateFile log

Comment 2

[Alfred Kayser]

See bug 269813 for some thoughts.

msChromeRegistry.cpp is suspicious:
(http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/rdf/chrome/src/nsChromeRegistry.cpp&rev=1.327#733)

 734                    //
 735                    // also cache the zip file itself
 736                    //
 737                    nsCOMPtr<nsIZipReaderCache> readerCache =
 738                      do_CreateInstance("@mozilla.org/libjar/zip-reader-cache;1", &rv);
 739                    if (NS_FAILED(rv)) return rv;
 740                  
 741                    rv = readerCache->Init(32);
 742                    
 743                    rv = readerCache->GetZip(overrideFile, getter_AddRefs(mOverrideJAR));
 744                    if (NS_FAILED(rv)) {
 745                      mOverrideJARURL.Truncate();
 746                      return rv;
 747                    }

This looks to me like for each request a readerCache is created and inited?
And only to check for existance of a certain entry in the jarfile?

Comment 3

[Alfred Kayser]

Issues to solve:
* nsIZipReaderCache returns a nsIZipReader which can be abused (init, close, etc), which confuses the jarcache (see bug 269813)
* I have seen an example on forumzilla where people create a jarcache with 1024 entries (meaning 1024 jarfiles!), thinking they are caching 1024 zipentries...
* nsIZipReaderCache is in the mozilla codebase only used in two locations: inside libjar itself, and in rdf/chrome.
* nsIZipReaderCache is in rdr/chrome wrongly used (see comment #2): Create cache, init, getzip, destroy cache...

So the whole the idea is to remove the public nsIZipReaderCache, and to add two functions:
   hasEntry(jarFile, entryName): to check for existance of an certain entry in a jarfile
and
   getInputStream(jarFile, entryName): to get the corresponding nsIInputStream (JARInputStream resp. JARDirectoryInputStream).

As the JARProtocolHandler allready contain a jarcache instance, and it is always inited, it would make sense to add these functions to nsIJARProtocolHandler.
This would also enable to merge the ZipReaderCache code into nsJARProtocolHandler.cpp.

Roadmap to fix/improve jar caching:
	1. Buildsyn: Correctly build the synthetized entries (bug 333129)
	2. Dirinput: Merge JARDirectoryInputStream with JARInputStream (bug 337561)
	3. Jarcache: Move jarcache from nsIZipReaderCache to nsIJARProtocolHandler (hasEntry, getInputStream)
	4. ZipEntry: Remove nsIZipEntry (bug 203627)

Comment 4

[Feng Qian (Google)]

(In reply to comment #0)
> JAR cache not working?

Per our discussion, a jar file opened more than once because it is opened first time when a nsJAR object for it was created. Whenever nsJAR:GetInputStream is called on an entry file name in the jar, the jar file is opened again and a new file descriptor is returned. It is necessary to allow multiple entry files be accessed at the same time.

> 
> I logged calls to CreateFile in NSPR, and I found that several JAR files are
> opened many times during startup.  However, our JAR files are supposed to only
> be opened once and cached for later use.
> 
> I'll attach my log of CreateFile calls shortly.
> 

Comment 5

[Darin Fisher]

Marking WORKSFORME based on comment #4