jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From alexparvule...@apache.org
Subject svn commit: r1205361 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene: ./ sort/
Date Wed, 23 Nov 2011 10:50:14 GMT
Author: alexparvulescu
Date: Wed Nov 23 10:50:12 2011
New Revision: 1205361

URL: http://svn.apache.org/viewvc?rev=1205361&view=rev
Log:
JCR-3151 SharedFieldCache can cause a memory leak

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/CachingMultiIndexReader.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorBase.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ReadOnlyIndexReader.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldComparatorSource.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/sort/AbstractFieldComparator.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/CachingMultiIndexReader.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/CachingMultiIndexReader.java?rev=1205361&r1=1205360&r2=1205361&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/CachingMultiIndexReader.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/CachingMultiIndexReader.java
Wed Nov 23 10:50:12 2011
@@ -165,6 +165,8 @@ public final class CachingMultiIndexRead
         for (ReadOnlyIndexReader subReader : subReaders) {
             subReader.release();
         }
+        subReaders = null;
+        readersByCreationTick.clear();
     }
 
     //-------------------------< MultiIndexReader >-----------------------------

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorBase.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorBase.java?rev=1205361&r1=1205360&r2=1205361&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorBase.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/FieldComparatorBase.java
Wed Nov 23 10:50:12 2011
@@ -30,7 +30,7 @@ abstract public class FieldComparatorBas
     /**
      * The bottom value.
      */
-    private Comparable bottom;
+    private Comparable<?> bottom;
 
     /**
      * Value for a document
@@ -38,7 +38,7 @@ abstract public class FieldComparatorBas
      * @param doc  id of the document
      * @return  the value for the given id
      */
-    protected abstract Comparable sortValue(int doc);
+    protected abstract Comparable<?> sortValue(int doc);
 
     /**
      * Retrieves the value of a given slot
@@ -46,7 +46,7 @@ abstract public class FieldComparatorBas
      * @param slot  index of the value to retrieve
      * @return  the value in the given slot
      */
-    protected abstract Comparable getValue(int slot);
+    protected abstract Comparable<?> getValue(int slot);
 
     /**
      * Puts a value into a given slot
@@ -54,7 +54,7 @@ abstract public class FieldComparatorBas
      * @param slot  index where to put the value
      * @param value  the value to put into the given slot
      */
-    protected abstract void setValue(int slot, Comparable value);
+    protected abstract void setValue(int slot, Comparable<?> value);
 
     @Override
     public int compare(int slot1, int slot2) {
@@ -80,7 +80,7 @@ abstract public class FieldComparatorBas
      *   a positive integer if <code>val1</code> comes after <code>val2</code>
and
      *   <code>0</code> if <code>val1</code> and <code>val2</code>
are equal.
      */
-    protected int compare(Comparable val1, Comparable val2) {
+    protected int compare(Comparable<?> val1, Comparable<?> val2) {
         if (val1 == null) {
             if (val2 == null) {
                 return 0;
@@ -99,7 +99,7 @@ abstract public class FieldComparatorBas
     }
 
     @Override
-    public Comparable value(int slot) {
+    public Comparable<?> value(int slot) {
         return getValue(slot);
     }
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ReadOnlyIndexReader.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ReadOnlyIndexReader.java?rev=1205361&r1=1205360&r2=1205361&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ReadOnlyIndexReader.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ReadOnlyIndexReader.java
Wed Nov 23 10:50:12 2011
@@ -33,11 +33,6 @@ import java.util.Map;
 class ReadOnlyIndexReader extends RefCountingIndexReader {
 
     /**
-     * The underlying shared reader.
-     */
-    private final SharedIndexReader reader;
-
-    /**
      * The deleted documents as initially read from the IndexReader passed
      * in the constructor of this class.
      */
@@ -63,7 +58,6 @@ class ReadOnlyIndexReader extends RefCou
                                BitSet deleted,
                                long deletedDocsVersion) {
         super(reader);
-        this.reader = reader;
         this.deleted = deleted;
         this.deletedDocsVersion = deletedDocsVersion;
         // acquire underlying reader
@@ -84,7 +78,7 @@ class ReadOnlyIndexReader extends RefCou
      * @return the creation tick for the underlying reader.
      */
     long getCreationTick() {
-        return reader.getCreationTick();
+        return getBase().getCreationTick();
     }
 
     /**
@@ -194,7 +188,7 @@ class ReadOnlyIndexReader extends RefCou
      */
     public TermDocs termDocs(Term term) throws IOException {
         // do not wrap for empty TermDocs
-        TermDocs td = reader.termDocs(term);
+        TermDocs td = in.termDocs(term);
         if (td != EmptyTermDocs.INSTANCE) {
             td = new FilteredTermDocs(td);
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java?rev=1205361&r1=1205360&r2=1205361&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldCache.java
Wed Nov 23 10:50:12 2011
@@ -16,18 +16,18 @@
  */
 package org.apache.jackrabbit.core.query.lucene;
 
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+import javax.jcr.PropertyType;
+
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.TermDocs;
 import org.apache.lucene.index.TermEnum;
 import org.apache.lucene.index.TermPositions;
-import org.apache.lucene.search.FieldComparator;
-
-import javax.jcr.PropertyType;
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.WeakHashMap;
 
 /**
  * Implements a variant of the lucene class <code>org.apache.lucene.search.FieldCacheImpl</code>.
@@ -52,12 +52,12 @@ public class SharedFieldCache {
         /**
          * Values indexed by document id.
          */
-        private final Comparable[] values;
+        private final Comparable<?>[] values;
 
         /**
          * Values (Comparable) map indexed by document id.
          */
-        public final Map<Integer, Comparable> valuesMap;
+        public final Map<Integer, Comparable<?>> valuesMap;
 
         /**
          * Boolean indicating whether the {@link #valuesMap} impl has to be used
@@ -67,7 +67,7 @@ public class SharedFieldCache {
         /**
          * Creates one of these objects
          */
-        public ValueIndex(Comparable[] values, int setValues) {
+        public ValueIndex(Comparable<?>[] values, int setValues) {
             if (isSparse(values, setValues)) {
                 this.sparse = true;
                 this.values = null;
@@ -83,7 +83,7 @@ public class SharedFieldCache {
             }
         }
 
-        public Comparable getValue(int i) {
+        public Comparable<?> getValue(int i) {
             if (sparse) {
                 return valuesMap == null ? null : valuesMap.get(i);
             } else {
@@ -91,8 +91,8 @@ public class SharedFieldCache {
             }
         }
 
-        private Map<Integer, Comparable> getValuesMap(Comparable[] values, int setValues)
{
-            Map<Integer, Comparable> map = new HashMap<Integer, Comparable>(setValues);
+        private static Map<Integer, Comparable<?>> getValuesMap(Comparable<?>[]
values, int setValues) {
+            Map<Integer, Comparable<?>> map = new HashMap<Integer, Comparable<?>>(setValues);
             for (int i = 0; i < values.length && setValues > 0; i++) {
                 if (values[i] != null) {
                     map.put(i, values[i]);
@@ -102,7 +102,7 @@ public class SharedFieldCache {
             return map;
         }
 
-        private boolean isSparse(Comparable[] values, int setValues) {
+        private static boolean isSparse(Comparable<?>[] values, int setValues) {
             // some really simple test to test whether the array is sparse. Currently, when
less then 1% is set, the array is already sparse 
             // for this typical cache to avoid memory issues
             if (setValues * SPARSE_FACTOR < values.length) {
@@ -138,25 +138,21 @@ public class SharedFieldCache {
      * @param reader     the <code>IndexReader</code>.
      * @param field      name of the shared field.
      * @param prefix     the property name, will be used as term prefix.
-     * @param comparator the field comparator instance.
      * @return a ValueIndex that contains the field values and order
      *         information.
      * @throws IOException if an error occurs while reading from the index.
      */
-    public ValueIndex getValueIndex(IndexReader reader,
-                                    String field,
-                                    String prefix,
-                                    FieldComparator comparator)
-            throws IOException {
+    public ValueIndex getValueIndex(IndexReader reader, String field,
+            String prefix) throws IOException {
 
         if (reader instanceof ReadOnlyIndexReader) {
             reader = ((ReadOnlyIndexReader) reader).getBase();
         }
 
         field = field.intern();
-        ValueIndex ret = lookup(reader, field, prefix, comparator);
+        ValueIndex ret = lookup(reader, field, prefix);
         if (ret == null) {
-            Comparable[] retArray = new Comparable[reader.maxDoc()];
+            Comparable<?>[] retArray = new Comparable[reader.maxDoc()];
             int setValues = 0;
             if (retArray.length > 0) {
                 IndexFormatVersion version = IndexFormatVersion.getVersion(reader);
@@ -214,7 +210,7 @@ public class SharedFieldCache {
                 }
             }
             ValueIndex value = new ValueIndex(retArray, setValues);
-            store(reader, field, prefix, comparator, value);
+            store(reader, field, prefix, value);
             return value;
         }
         return ret;
@@ -223,31 +219,27 @@ public class SharedFieldCache {
     /**
      * See if a <code>ValueIndex</code> object is in the cache.
      */
-    ValueIndex lookup(IndexReader reader, String field,
-                      String prefix, FieldComparator comparer) {
-        Key key = new Key(field, prefix, comparer);
-        synchronized (this) {
+    ValueIndex lookup(IndexReader reader, String field, String prefix) {
+        synchronized (cache) {
             Map<Key, ValueIndex> readerCache = cache.get(reader);
             if (readerCache == null) {
                 return null;
             }
-            return readerCache.get(key);
+            return readerCache.get(new Key(field, prefix));
         }
     }
 
     /**
      * Put a <code>ValueIndex</code> <code>value</code> to cache.
      */
-    ValueIndex store(IndexReader reader, String field, String prefix,
-                 FieldComparator comparer, ValueIndex value) {
-        Key key = new Key(field, prefix, comparer);
-        synchronized (this) {
+    void store(IndexReader reader, String field, String prefix, ValueIndex value) {
+        synchronized (cache) {
             Map<Key, ValueIndex> readerCache = cache.get(reader);
             if (readerCache == null) {
                 readerCache = new HashMap<Key, ValueIndex>();
                 cache.put(reader, readerCache);
             }
-            return readerCache.put(key, value);
+            readerCache.put(new Key(field, prefix), value);
         }
     }
 
@@ -259,7 +251,7 @@ public class SharedFieldCache {
      * @param type the property type.
      * @return a comparable for the <code>value</code>.
      */
-    private Comparable getValue(String value, int type) {
+    private Comparable<?> getValue(String value, int type) {
         switch (type) {
             case PropertyType.BOOLEAN:
                 return Boolean.valueOf(value);
@@ -278,42 +270,39 @@ public class SharedFieldCache {
 
     /**
      * A compound <code>Key</code> that consist of <code>field</code>
-     * <code>prefix</code> and <code>comparator</code>.
+     * and <code>prefix</code>.
      */
     static class Key {
 
         private final String field;
         private final String prefix;
-        private final Object comparator;
 
         /**
          * Creates <code>Key</code> for ValueIndex lookup.
          */
-        Key(String field, String prefix, FieldComparator comparator) { 
+        Key(String field, String prefix) { 
             this.field = field.intern();
             this.prefix = prefix.intern();
-            this.comparator = comparator;
         }
 
         /**
          * Returns <code>true</code> if <code>o</code> is a <code>Key</code>
-         * instance and refers to the same field, prefix and comparator object.
+         * instance and refers to the same field and prefix.
          */
         public boolean equals(Object o) {
             if (o instanceof Key) {
                 Key other = (Key) o;
                 return other.field == field
-                        && other.prefix == prefix
-                        && other.comparator.equals(comparator);
+                        && other.prefix == prefix;
             }
             return false;
         }
 
         /**
-         * Composes a hashcode based on the field, prefix and comparator.
+         * Composes a hashcode based on the field and prefix.
          */
         public int hashCode() {
-            return field.hashCode() ^ prefix.hashCode() ^ comparator.hashCode();
+            return field.hashCode() ^ prefix.hashCode();
         }
     }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldComparatorSource.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldComparatorSource.java?rev=1205361&r1=1205360&r2=1205361&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldComparatorSource.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SharedFieldComparatorSource.java
Wed Nov 23 10:50:12 2011
@@ -17,6 +17,8 @@
 
 package org.apache.jackrabbit.core.query.lucene;
 
+import java.io.IOException;
+
 import org.apache.jackrabbit.core.HierarchyManager;
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.id.PropertyId;
@@ -29,19 +31,18 @@ import org.apache.jackrabbit.spi.PathFac
 import org.apache.jackrabbit.spi.commons.conversion.IllegalNameException;
 import org.apache.jackrabbit.spi.commons.name.PathBuilder;
 import org.apache.jackrabbit.spi.commons.name.PathFactoryImpl;
-import org.apache.lucene.document.Document;
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.search.FieldComparator;
 import org.apache.lucene.search.FieldComparatorSource;
 
-import java.io.IOException;
-
 /**
  * Implements a <code>FieldComparatorSource</code> for <code>FieldComparator</code>s
which
  * know how to sort on a lucene field that contains values for multiple properties.
  */
 public class SharedFieldComparatorSource extends FieldComparatorSource {
 
+    private static final long serialVersionUID = -5803240954874585429L;
+
     /**
      * The name of the shared field in the lucene index.
      */
@@ -151,12 +152,13 @@ public class SharedFieldComparatorSource
             String namedValue = FieldNames.createNamedValue(propertyName, "");
             for (int i = 0; i < readers.size(); i++) {
                 IndexReader r = readers.get(i);
-                indexes[i] = SharedFieldCache.INSTANCE.getValueIndex(r, fieldName, namedValue,
this);
+                indexes[i] = SharedFieldCache.INSTANCE.getValueIndex(r,
+                        fieldName, namedValue);
             }
         }
 
         @Override
-        protected Comparable sortValue(int doc) {
+        protected Comparable<?> sortValue(int doc) {
             int idx = readerIndex(doc);
             return indexes[idx].getValue(doc - starts[idx]);
         }
@@ -186,7 +188,7 @@ public class SharedFieldComparatorSource
         }
 
         @Override
-        protected Comparable sortValue(int doc) {
+        protected Comparable<?> sortValue(int doc) {
             try {
                 final String uuid = getUUIDForIndex(doc);
                 
@@ -236,10 +238,10 @@ public class SharedFieldComparatorSource
         }
 
         @Override
-        public Comparable sortValue(int doc) {
+        public Comparable<?> sortValue(int doc) {
             for (FieldComparator fieldComparator : fieldComparators) {
                 if (fieldComparator instanceof FieldComparatorBase) {
-                    Comparable c = ((FieldComparatorBase) fieldComparator).sortValue(doc);
+                    Comparable<?> c = ((FieldComparatorBase) fieldComparator).sortValue(doc);
 
                     if (c != null) {
                         return c;

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/sort/AbstractFieldComparator.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/sort/AbstractFieldComparator.java?rev=1205361&r1=1205360&r2=1205361&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/sort/AbstractFieldComparator.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/sort/AbstractFieldComparator.java
Wed Nov 23 10:50:12 2011
@@ -35,7 +35,7 @@ public abstract class AbstractFieldCompa
     /**
      * The values for comparing.
      */
-    private final Comparable[] values;
+    private final Comparable<?>[] values;
 
     /**
      * The index readers.
@@ -92,7 +92,7 @@ public abstract class AbstractFieldCompa
      * @param value  value for adding
      */
     @Override
-    public void setValue(int slot, Comparable value) {
+    public void setValue(int slot, Comparable<?> value) {
         values[slot] = value;
     }
 
@@ -103,7 +103,7 @@ public abstract class AbstractFieldCompa
      * @return  the retrieved value
      */
     @Override
-    public Comparable getValue(int slot) {
+    public Comparable<?> getValue(int slot) {
         return values[slot];
     }
 



Mime
View raw message