Index: lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextPerDocProducer.java =================================================================== --- lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextPerDocProducer.java (revision 1406079) +++ lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextPerDocProducer.java (working copy) @@ -136,7 +136,7 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { boolean success = false; IndexInput in = input.clone(); try { @@ -198,9 +198,14 @@ assert scratch.equals(END); return reader.getSource(); } + + @Override + public Source getDirectSource() throws IOException { + return this.getSource(); // don't cache twice + } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return this.getSource(); } Index: lucene/core/src/test/org/apache/lucene/index/TestDocValuesIndexing.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestDocValuesIndexing.java (revision 1406079) +++ lucene/core/src/test/org/apache/lucene/index/TestDocValuesIndexing.java (working copy) @@ -47,7 +47,9 @@ import org.apache.lucene.document.TextField; import org.apache.lucene.index.DocValues.SortedSource; import org.apache.lucene.index.DocValues.Source; +import org.apache.lucene.index.DocValues.SourceCache; import org.apache.lucene.index.DocValues.Type; +import org.apache.lucene.index.DocValues.SourceCache.DirectSourceCache; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.DocIdSetIterator; @@ -576,15 +578,26 @@ } private DocValues getDocValues(IndexReader reader, String field) throws IOException { - return MultiDocValues.getDocValues(reader, field); - } + final DocValues docValues = MultiDocValues.getDocValues(reader, field); + if (docValues == null) { + return docValues; + } + if (rarely()) { + docValues.setCache(new NotCachingSourceCache()); + } else { + if (!(docValues.getCache() instanceof DirectSourceCache)) { + docValues.setCache(new DirectSourceCache()); + } + } + return docValues; + } @SuppressWarnings("fallthrough") private Source getSource(DocValues values) throws IOException { // getSource uses cache internally switch(random().nextInt(5)) { case 3: - return values.load(); + return values.loadSource(); case 2: return values.getDirectSource(); case 1: @@ -764,7 +777,7 @@ w.forceMerge(1); DirectoryReader r = w.getReader(); w.close(); - assertEquals(17, getOnlySegmentReader(r).docValues("field").load().getInt(0)); + assertEquals(17, getOnlySegmentReader(r).docValues("field").loadSource().getInt(0)); r.close(); d.close(); } @@ -791,7 +804,7 @@ w.forceMerge(1); DirectoryReader r = w.getReader(); w.close(); - assertEquals(17, getOnlySegmentReader(r).docValues("field").load().getInt(0)); + assertEquals(17, getOnlySegmentReader(r).docValues("field").loadSource().getInt(0)); r.close(); d.close(); } @@ -1072,4 +1085,24 @@ writer.close(); dir.close(); } + + /** + * + */ + public static class NotCachingSourceCache extends SourceCache { + + @Override + public Source load(DocValues values) throws IOException { + return values.loadSource(); + } + + @Override + public Source loadDirect(DocValues values) throws IOException { + return values.loadDirectSource(); + } + + @Override + public void invalidate(DocValues values) {} + } + } Index: lucene/core/src/test/org/apache/lucene/codecs/lucene40/values/TestDocValues.java =================================================================== --- lucene/core/src/test/org/apache/lucene/codecs/lucene40/values/TestDocValues.java (revision 1406079) +++ lucene/core/src/test/org/apache/lucene/codecs/lucene40/values/TestDocValues.java (working copy) @@ -425,8 +425,6 @@ private Source getSource(DocValues values) throws IOException { // getSource uses cache internally switch(random().nextInt(5)) { - case 3: - return values.load(); case 2: return values.getDirectSource(); case 1: Index: lucene/core/src/java/org/apache/lucene/index/MultiDocValues.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/MultiDocValues.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/index/MultiDocValues.java (working copy) @@ -185,7 +185,7 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return new MultiSource(slices, starts, false, type); } @@ -199,7 +199,7 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return emptySource; } @@ -209,7 +209,7 @@ } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return emptySource; } } @@ -226,7 +226,7 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return emptyFixedSource; } @@ -241,7 +241,7 @@ } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return emptyFixedSource; } } @@ -594,7 +594,7 @@ } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return new MultiSource(slices, starts, true, type); } Index: lucene/core/src/java/org/apache/lucene/index/DocValues.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/DocValues.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/index/DocValues.java (working copy) @@ -33,8 +33,8 @@ import org.apache.lucene.document.ShortDocValuesField; // javadocs import org.apache.lucene.document.SortedBytesDocValuesField; // javadocs import org.apache.lucene.document.StraightBytesDocValuesField; // javadocs -import org.apache.lucene.store.DataOutput; import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.CloseableThreadLocal; import org.apache.lucene.util.packed.PackedInts; /** @@ -95,7 +95,6 @@ private volatile SourceCache cache = new SourceCache.DirectSourceCache(); private final Object cacheLock = new Object(); - /** Sole constructor. (For invocation by subclass * constructors, typically implicit.) */ protected DocValues() { @@ -112,12 +111,12 @@ * @see #getSource() * @see #setCache(SourceCache) */ - public abstract Source load() throws IOException; + protected abstract Source loadSource() throws IOException; /** * Returns a {@link Source} instance through the current {@link SourceCache}. * Iff no {@link Source} has been loaded into the cache so far the source will - * be loaded through {@link #load()} and passed to the {@link SourceCache}. + * be loaded through {@link #loadSource()} and passed to the {@link SourceCache}. * The caller of this method should not close the obtained {@link Source} * instance unless it is not needed for the rest of its life time. *

@@ -129,12 +128,30 @@ public Source getSource() throws IOException { return cache.load(this); } + + /** + * Returns a disk resident {@link Source} instance through the current + * {@link SourceCache}. Direct Sources are cached per thread in the + * {@link SourceCache}. The obtained instance should not be shared with other + * threads. + */ + public Source getDirectSource() throws IOException { + return this.cache.loadDirect(this); + } + /** - * Returns a disk resident {@link Source} instance. Direct Sources are not - * cached in the {@link SourceCache} and should not be shared between threads. + * Loads a new {@link Source direct source} instance from this {@link DocValues} field + * instance. Source instances returned from this method are not cached. It is + * the callers responsibility to maintain the instance and release its + * resources once the source is not needed anymore. + *

+ * For managed {@link Source direct source} instances see {@link #getDirectSource()}. + * + * @see #getDirectSource() + * @see #setCache(SourceCache) */ - public abstract Source getDirectSource() throws IOException; + protected abstract Source loadDirectSource() throws IOException; /** * Returns the {@link Type} of this {@link DocValues} instance @@ -163,10 +180,10 @@ /** * Sets the {@link SourceCache} used by this {@link DocValues} instance. This - * method should be called before {@link #load()} is called. All {@link Source} instances in the currently used cache will be closed + * method should be called before {@link #loadSource()} is called. All {@link Source} instances in the currently used cache will be closed * before the new cache is installed. *

- * Note: All instances previously obtained from {@link #load()} will be lost. + * Note: All instances previously obtained from {@link #loadSource()} will be lost. * * @throws IllegalArgumentException * if the given cache is null @@ -181,6 +198,14 @@ toClose.close(this); } } + /** + * Returns the currently used cache instance; + * @see #setCache(SourceCache) + */ + // for tests + SourceCache getCache() { + return cache; + } /** * Source of per document values like long, double or {@link BytesRef} @@ -687,9 +712,9 @@ /** * Abstract base class for {@link DocValues} {@link Source} cache. *

- * {@link Source} instances loaded via {@link DocValues#load()} are entirely memory resident + * {@link Source} instances loaded via {@link DocValues#loadSource()} are entirely memory resident * and need to be maintained by the caller. Each call to - * {@link DocValues#load()} will cause an entire reload of + * {@link DocValues#loadSource()} will cause an entire reload of * the underlying data. Source instances obtained from * {@link DocValues#getSource()} and {@link DocValues#getSource()} * respectively are maintained by a {@link SourceCache} that is closed ( @@ -721,6 +746,15 @@ * This method will not return null */ public abstract Source load(DocValues values) throws IOException; + + /** + * Atomically loads a {@link Source direct source} into the per-thread cache from the given + * {@link DocValues} and returns it iff no other {@link Source direct source} has already + * been cached. Otherwise the cached source is returned. + *

+ * This method will not return null + */ + public abstract Source loadDirect(DocValues values) throws IOException; /** * Atomically invalidates the cached {@link Source} @@ -744,21 +778,35 @@ */ public static final class DirectSourceCache extends SourceCache { private Source ref; - + private final CloseableThreadLocal directSourceCache = new CloseableThreadLocal(); + /** Sole constructor. */ public DirectSourceCache() { } public synchronized Source load(DocValues values) throws IOException { if (ref == null) { - ref = values.load(); + ref = values.loadSource(); } return ref; } public synchronized void invalidate(DocValues values) { ref = null; + directSourceCache.close(); } + + @Override + public synchronized Source loadDirect(DocValues values) throws IOException { + final Source source = directSourceCache.get(); + if (source == null) { + final Source loadDirectSource = values.loadDirectSource(); + directSourceCache.set(loadDirectSource); + return loadDirectSource; + } else { + return source; + } + } } } } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedStraightBytesImpl.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedStraightBytesImpl.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedStraightBytesImpl.java (working copy) @@ -280,7 +280,7 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return size == 1 ? new SingleByteSource(cloneData(), maxDoc) : new FixedStraightSource(cloneData(), size, maxDoc, type); } @@ -291,7 +291,7 @@ } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return new DirectFixedStraightSource(cloneData(), size, getType()); } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedDerefBytesImpl.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedDerefBytesImpl.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedDerefBytesImpl.java (working copy) @@ -79,12 +79,12 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return new FixedDerefSource(cloneData(), cloneIndex(), size, numValuesStored); } @Override - public Source getDirectSource() + protected Source loadDirectSource() throws IOException { return new DirectFixedDerefSource(cloneData(), cloneIndex(), size, getType()); } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarSortedBytesImpl.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarSortedBytesImpl.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarSortedBytesImpl.java (working copy) @@ -161,13 +161,13 @@ } @Override - public org.apache.lucene.index.DocValues.Source load() + public org.apache.lucene.index.DocValues.Source loadSource() throws IOException { return new VarSortedSource(cloneData(), cloneIndex(), comparator); } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return new DirectSortedSource(cloneData(), cloneIndex(), comparator, getType()); } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedSortedBytesImpl.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedSortedBytesImpl.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/FixedSortedBytesImpl.java (working copy) @@ -135,13 +135,13 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return new FixedSortedSource(cloneData(), cloneIndex(), size, valueCount, comparator); } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return new DirectFixedSortedSource(cloneData(), cloneIndex(), size, valueCount, comparator, type); } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Ints.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Ints.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Ints.java (working copy) @@ -149,7 +149,7 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { final IndexInput indexInput = cloneData(); try { return arrayTemplate.newFromInput(indexInput, maxDoc); Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/PackedIntValues.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/PackedIntValues.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/PackedIntValues.java (working copy) @@ -149,7 +149,7 @@ /** * Opens all necessary files, but does not read any data in until you call - * {@link #load}. + * {@link #loadSource}. */ static class PackedIntsReader extends DocValues { private final IndexInput datIn; @@ -182,7 +182,7 @@ * already previously loaded but then discarded the Source. */ @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { boolean success = false; final Source source; IndexInput input = null; @@ -217,7 +217,7 @@ @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return values != null ? new FixedStraightBytesImpl.DirectFixedStraightSource(datIn.clone(), 8, Type.FIXED_INTS_64) : new PackedIntsSource(datIn.clone(), true); } } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Bytes.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Bytes.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Bytes.java (working copy) @@ -308,7 +308,7 @@ /** * Opens all necessary files, but does not read any data in until you call - * {@link #load}. + * {@link #loadSource}. */ static abstract class BytesReaderBase extends DocValues { protected final IndexInput idxIn; Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarStraightBytesImpl.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarStraightBytesImpl.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarStraightBytesImpl.java (working copy) @@ -247,12 +247,12 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return new VarStraightSource(cloneData(), cloneIndex()); } @Override - public Source getDirectSource() + protected Source loadDirectSource() throws IOException { return new DirectVarStraightSource(cloneData(), cloneIndex(), getType()); } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarDerefBytesImpl.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarDerefBytesImpl.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/VarDerefBytesImpl.java (working copy) @@ -99,12 +99,12 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return new VarDerefSource(cloneData(), cloneIndex(), totalBytes); } @Override - public Source getDirectSource() + protected Source loadDirectSource() throws IOException { return new DirectVarDerefSource(cloneData(), cloneIndex(), getType()); } Index: lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Floats.java =================================================================== --- lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Floats.java (revision 1406079) +++ lucene/core/src/java/org/apache/lucene/codecs/lucene40/values/Floats.java (working copy) @@ -125,7 +125,7 @@ } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { final IndexInput indexInput = cloneData(); try { return arrayTemplate.newFromInput(indexInput, maxDoc); Index: lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndexNormDocValues.java =================================================================== --- lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndexNormDocValues.java (revision 1406079) +++ lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndexNormDocValues.java (working copy) @@ -33,12 +33,12 @@ this.source = source; } @Override - public Source load() throws IOException { + protected Source loadSource() throws IOException { return source; } @Override - public Source getDirectSource() throws IOException { + protected Source loadDirectSource() throws IOException { return source; } Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 1406079) +++ lucene/CHANGES.txt (working copy) @@ -161,6 +161,10 @@ posting lists. All index data is represented as consecutive byte/int arrays to reduce GC cost and memory overhead. (Simon Willnauer) +* LUCENE-4538: DocValues now caches direct sources in a ThreadLocal exposed via SourceCache. + Users of this API can now simply obtain an instance via DocValues#getDirectSource per thread. + (Simon Willnauer) + Build * Upgrade randomized testing to version 2.0.4: avoid hangs on shutdown