Trying to resolve [Issue-5], unintended object retention causing OOME

Author: cowtowncoder

pom.xml

@@ -3,12 +3,12 @@
   <parent>
     <groupId>org.sonatype.oss</groupId>
     <artifactId>oss-parent</artifactId>
-    <version>5</version>
+    <version>7</version>
   </parent>
   <groupId>com.fasterxml.util</groupId>
   <artifactId>java-merge-sort</artifactId>
   <name>java-merge-sort</name>
-  <version>0.6.1-SNAPSHOT</version>
+  <version>0.7.0-SNAPSHOT</version>
   <packaging>bundle</packaging>
   <description>Basic disk-backed N-way merge sort
   </description>
@@ -119,11 +119,11 @@
         <plugin>
           <groupId>org.apache.felix</groupId>
           <artifactId>maven-bundle-plugin</artifactId>
-          <version>2.3.4</version>
+          <version>2.3.7</version>
           <extensions>true</extensions>
           <configuration>
             <instructions>
-              <Bundle-SymbolicName>${project.artifactId}</Bundle-SymbolicName>
+              <Bundle-SymbolicName>${project.groupId}.${project.artifactId}</Bundle-SymbolicName>
               <Bundle-Vendor>com.fasterxml</Bundle-Vendor>
               <Import-Package>
 </Import-Package>

release-notes/VERSION

@@ -0,0 +1,19 @@
+Version: 0.7.0
+
+Release date:
+  29-May-2012
+
+Description:
+  Another pre-1.0 release
+
+Fixes:
+
+* [Issue-5]: Unnecessary object retention, leading to too high memory usage
+ (reported by Paul Smith)
+
+------------------------------------------------------------------------
+=== History: ===
+------------------------------------------------------------------------
+
+2.0.2 (16-May-2012)
+

src/main/java/com/fasterxml/sort/Sorter.java

@@ -195,16 +195,12 @@ public void sort(InputStream source, OutputStream destination)
      * 
      * @return true if sorting completed succesfully; false if it was cancelled
      */
-    public boolean sort(DataReader<T>  inputReader, DataWriter<T> resultWriter)
+    public boolean sort(DataReader<T> inputReader, DataWriter<T> resultWriter)
         throws IOException
     {
         // First, pre-sort:
         _phase = SortingState.Phase.PRE_SORTING;
 
-        /* Minor optimization: in case all entries might fit in
-         * in-memory sort buffer, avoid writing intermediate file
-         * and just write results directly.
-         */
         SegmentedBuffer buffer = new SegmentedBuffer();
         boolean inputClosed = false;
         boolean resultClosed = false;
@@ -220,6 +216,10 @@ public boolean sort(DataReader<T>  inputReader, DataWriter<T> resultWriter)
             }
             Arrays.sort(items, _rawComparator());
             T next = inputReader.readNext();
+            /* Minor optimization: in case all entries might fit in
+             * in-memory sort buffer, avoid writing intermediate file
+             * and just write results directly.
+             */
             if (next == null) {
                 inputClosed = true;
                 inputReader.close();
@@ -318,6 +318,8 @@ protected List<File> presort(DataReader<T> inputReader,
     {
         ArrayList<File> presorted = new ArrayList<File>();
         presorted.add(_writePresorted(firstSortedBatch));
+        // important: clear out the ref to let possibly sizable array to be GCed
+        firstSortedBatch = null;
         do {
             Object[] items = _readMax(inputReader, buffer, _config.getMaxMemoryUsage(), nextValue);
             Arrays.sort(items, _rawComparator());
@@ -333,8 +335,10 @@ protected File _writePresorted(Object[] items) throws IOException
         @SuppressWarnings("unchecked")
         DataWriter<Object> writer = (DataWriter<Object>) _writerFactory.constructWriter(new FileOutputStream(tmp));
         ++_presortFileCount;
-        for (Object item : items) {
-            writer.writeEntry(item);
+        for (int i = 0, end = items.length; i < end; ++i) {
+            writer.writeEntry(items[i]);
+            // to further reduce transient mem usage, clear out the ref
+            items[i] = null;
         }
         writer.close();
         return tmp;

src/main/java/com/fasterxml/sort/util/SegmentedBuffer.java

@@ -1,5 +1,7 @@
 package com.fasterxml.sort.util;
 
+import java.util.Arrays;
+
 /**
  * Helper class used instead of a standard JDK list or buffer,
  * to avoid constant re-allocations.
@@ -59,7 +61,9 @@ public SegmentedBuffer() { }
      */
     public Object[] resetAndStart()
     {
-        _reset();
+        if (_bufferedEntryCount > 0) {
+            _reset();
+        }
         if (_freeBuffer == null) {
             return new Object[INITIAL_CHUNK_SIZE];
         }
@@ -115,6 +119,8 @@ public Object[] completeAndClearBuffer(Object[] lastChunk, int lastChunkEntries)
         int totalSize = lastChunkEntries + _bufferedEntryCount;
         Object[] result = new Object[totalSize];
         _copyTo(result, totalSize, lastChunk, lastChunkEntries);
+        // [Issue-5]: should reduce mem usage here
+        _reset();
         return result;
     }
 
@@ -144,12 +150,17 @@ public int initialCapacity()
     private void _reset()
     {
         // can we reuse the last (and thereby biggest) array for next time?
-        if (_bufferTail != null) {
-            _freeBuffer = _bufferTail.getData();
+        if (_bufferedEntryCount > 0) {
+            if (_bufferTail != null) {
+                Object[] obs = _bufferTail.getData();
+                // also, let's clear it of contents as well, just in case
+                Arrays.fill(obs, null);
+                _freeBuffer = obs;
+            }
+            // either way, must discard current contents
+            _bufferHead = _bufferTail = null;
+            _bufferedEntryCount = 0;
         }
-        // either way, must discard current contents
-        _bufferHead = _bufferTail = null;
-        _bufferedEntryCount = 0;
     }
 
     private final void _copyTo(Object resultArray, int totalSize,
@@ -186,9 +197,9 @@ private final static class Node
         /**
          * Data stored in this node. Array is considered to be full.
          */
-        final Object[] _data;
+        private final Object[] _data;
 
-        Node _next;
+        private Node _next;
 
         public Node(Object[] data) {
             _data = data;