8151807: ImageBufferCache should release buffers when all classes are loaded
authorplevart
Wed, 13 Apr 2016 09:35:26 -0300
changeset 37342 3f54fbfc2706
parent 37341 29f3a46ac3cc
child 37343 35a2231828a7
8151807: ImageBufferCache should release buffers when all classes are loaded Reviewed-by: jlaskey Contributed-by: peter.levart@gmail.com
jdk/src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java
jdk/src/java.base/share/classes/jdk/internal/jimage/ImageBufferCache.java
--- a/jdk/src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java	Wed Apr 13 10:41:27 2016 +0000
+++ b/jdk/src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java	Wed Apr 13 09:35:26 2016 -0300
@@ -288,22 +288,24 @@
 
             return buffer;
         } else {
+            if (channel == null) {
+                throw new InternalError("Image file channel not open");
+            }
+
             ByteBuffer buffer = ImageBufferCache.getBuffer(size);
-            int read = 0;
-
+            int read;
             try {
-                if (channel == null) {
-                    throw new InternalError("Image file channel not open");
-                }
-
                 read = channel.read(buffer, offset);
                 buffer.rewind();
             } catch (IOException ex) {
+                ImageBufferCache.releaseBuffer(buffer);
                 throw new RuntimeException(ex);
             }
 
             if (read != size) {
                 ImageBufferCache.releaseBuffer(buffer);
+                throw new RuntimeException("Short read: " + read +
+                                           " instead of " + size + " bytes");
             }
 
             return buffer;
--- a/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageBufferCache.java	Wed Apr 13 10:41:27 2016 +0000
+++ b/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageBufferCache.java	Wed Apr 13 09:35:26 2016 -0300
@@ -24,110 +24,112 @@
  */
 package jdk.internal.jimage;
 
+import java.lang.ref.WeakReference;
 import java.nio.ByteBuffer;
-import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Comparator;
 
 /**
  * @implNote This class needs to maintain JDK 8 source compatibility.
  *
  * It is used internally in the JDK to implement jimage/jrtfs access,
- * but also compiled and delivered as part of the jrtfs.jar to support access
+ * but also compiled and delivered as part of the jrt-fs.jar to support access
  * to the jimage file provided by the shipped JDK by tools running on JDK 8.
  */
 class ImageBufferCache {
-    private static final int MAX_FREE_BUFFERS = 3;
+    private static final int MAX_CACHED_BUFFERS = 3;
     private static final int LARGE_BUFFER = 0x10000;
-    private static final ThreadLocal<ArrayList<ImageBufferCache>>
-            threadLocal = new ThreadLocal<>();
+    private static final ThreadLocal<BufferReference[]> CACHE =
+        new ThreadLocal<BufferReference[]>() {
+            @Override
+            protected BufferReference[] initialValue() {
+                // 1 extra slot to simplify logic of releaseBuffer()
+                return new BufferReference[MAX_CACHED_BUFFERS + 1];
+            }
+        };
 
-    private final ByteBuffer buffer;
-    private boolean isUsed;
+    private static ByteBuffer allocateBuffer(long size) {
+        return ByteBuffer.allocateDirect((int)((size + 0xFFF) & ~0xFFF));
+    }
 
     static ByteBuffer getBuffer(long size) {
         if (size < 0 || Integer.MAX_VALUE < size) {
             throw new IndexOutOfBoundsException("size");
         }
 
-        ByteBuffer buffer = null;
+        ByteBuffer result = null;
 
         if (size > LARGE_BUFFER) {
-            buffer = ByteBuffer.allocateDirect((int)((size + 0xFFF) & ~0xFFF));
+            result = allocateBuffer(size);
         } else {
-            ArrayList<ImageBufferCache> buffers = threadLocal.get();
+            BufferReference[] cache = CACHE.get();
+
+            // buffers are ordered by decreasing capacity
+            // cache[MAX_CACHED_BUFFERS] is always null
+            for (int i = MAX_CACHED_BUFFERS - 1; i >= 0; i--) {
+                BufferReference reference = cache[i];
 
-            if (buffers == null) {
-                buffers = new ArrayList<>(MAX_FREE_BUFFERS);
-                threadLocal.set(buffers);
+                if (reference != null) {
+                    ByteBuffer buffer = reference.get();
+
+                    if (buffer != null && size <= buffer.capacity()) {
+                        cache[i] = null;
+                        result = buffer;
+                        result.rewind();
+                        break;
+                    }
+                }
             }
 
-            int i = 0, j = buffers.size();
-            for (ImageBufferCache imageBuffer : buffers) {
-                if (size <= imageBuffer.capacity()) {
-                    j = i;
-
-                    if (!imageBuffer.isUsed) {
-                        imageBuffer.isUsed = true;
-                        buffer = imageBuffer.buffer;
-
-                        break;
-                    }
-                } else {
-                    break;
-                }
-
-                i++;
-            }
-
-            if (buffer == null) {
-                ImageBufferCache imageBuffer = new ImageBufferCache((int)size);
-                buffers.add(j, imageBuffer);
-                buffer = imageBuffer.buffer;
+            if (result == null) {
+                result = allocateBuffer(size);
             }
         }
 
-        buffer.rewind();
-        buffer.limit((int)size);
+        result.limit((int)size);
 
-        return buffer;
+        return result;
     }
 
     static void releaseBuffer(ByteBuffer buffer) {
-        ArrayList<ImageBufferCache> buffers = threadLocal.get();
-
-        if (buffers == null) {
-            return;
-        }
-
         if (buffer.capacity() > LARGE_BUFFER) {
             return;
         }
 
-        int i = 0, j = buffers.size();
-        for (ImageBufferCache imageBuffer : buffers) {
-            if (!imageBuffer.isUsed) {
-                j = Math.min(j, i);
-            }
+        BufferReference[] cache = CACHE.get();
 
-            if (imageBuffer.buffer == buffer) {
-                imageBuffer.isUsed = false;
-                j = Math.min(j, i);
-
-                break;
+        // expunge cleared BufferRef(s)
+        for (int i = 0; i < MAX_CACHED_BUFFERS; i++) {
+            BufferReference reference = cache[i];
+            if (reference != null && reference.get() == null) {
+                cache[i] = null;
             }
         }
 
-        if (buffers.size() > MAX_FREE_BUFFERS && j != buffers.size()) {
-            buffers.remove(j);
-        }
+        // insert buffer back with new BufferRef wrapping it
+        cache[MAX_CACHED_BUFFERS] = new BufferReference(buffer);
+        Arrays.sort(cache, DECREASING_CAPACITY_NULLS_LAST);
+        // squeeze the smallest one out
+        cache[MAX_CACHED_BUFFERS] = null;
     }
 
-    private ImageBufferCache(int needed) {
-        this.buffer = ByteBuffer.allocateDirect((needed + 0xFFF) & ~0xFFF);
-        this.isUsed = true;
-        this.buffer.limit(needed);
-    }
+    private static Comparator<BufferReference> DECREASING_CAPACITY_NULLS_LAST =
+        new Comparator<BufferReference>() {
+            @Override
+            public int compare(BufferReference br1, BufferReference br2) {
+                return Integer.compare(br2 == null ? 0 : br2.capacity,
+                                       br1 == null ? 0 : br1.capacity);
+            }
+        };
 
-    private long capacity() {
-        return buffer.capacity();
+    private static class BufferReference extends WeakReference<ByteBuffer> {
+        // saved capacity so that DECREASING_CAPACITY_NULLS_LAST comparator
+        // is stable in the presence of GC clearing the WeakReference concurrently
+        final int capacity;
+
+        BufferReference(ByteBuffer buffer) {
+            super(buffer);
+            capacity = buffer.capacity();
+        }
     }
 }