Martin,
Given we now only cache one deflater per Zip/JarFile object, is
WeakReference here really
necessary? Basically wr is based on the vm heap memory and deflater is a
native memory,
arguably we are using the wrong measurement to decide whether or not to
give up the
deflater's native memory. Given someone (classloader) is keeping
hundreds and thousands
of jar/zip files alive, is it reasonable to assume it'd better to keep
the 32k cache for each
of them?
-Sherman
On 5/19/16 8:00 AM, Martin Buchholz wrote:
On Thu, May 19, 2016 at 7:29 AM, Peter Levart <peter.lev...@gmail.com> wrote:
But I have reservation for the implementation of one-element global cache of
Inflater. This cache can't be efficient. In order for cache to be efficient,
majority of calls to ZipFile.getInputStream(zipEntry) would have to be
accompanied by a corresponding explicit close() for the input stream before
the WeakReference to the cached Inflater is cleared.
That's my assumption. In most cases, failure to close something that
can be closed is a bug.
If there's code in the JDK that fails to do that, it should be fixed
independently.
The "assert !inf.ended()" in
releaseInflater() can therefore fail as final() methods on individual
objects that are eligible for finalization may be invoked in arbitrary
order.
Yeah, that's a bug. We can only assert after we verify that the
Inflater is still weakly reachable.
Updating my webrev with:
* Releases the Inflater for possible later reuse.
*/
private static void releaseInflater(Inflater inf) {
- assert !inf.ended();
CachedInflater cachedInflater = inflaterCache.get();
if (inf != cachedInflater.get()) {
inf.end();
} else {
// "return" the Inflater to the "pool".
+ assert !inf.ended();
inf.reset();
assert cachedInflater.inUse.get();
cachedInflater.inUse.set(false);
