Index: lucene/src/java/org/apache/lucene/index/FilterIndexReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/FilterIndexReader.java (revision 1214545) +++ lucene/src/java/org/apache/lucene/index/FilterIndexReader.java (working copy) @@ -388,16 +388,25 @@ return in.fields(); } - /** If the subclass of FilteredIndexReader modifies the - * contents of the FieldCache, you must override this - * method to provide a different key */ + /** {@inheritDoc} + *

If the subclass of FilteredIndexReader modifies the + * contents (but not liveDocs) of the index, you must override this + * method to provide a different key. */ @Override public Object getCoreCacheKey() { return in.getCoreCacheKey(); } - /** {@inheritDoc} */ + /** {@inheritDoc} + *

If the subclass of FilteredIndexReader modifies the + * liveDocs, you must override this + * method to provide a different key. */ @Override + public Object getCombinedCoreAndDeletesKey() { + return in.getCombinedCoreAndDeletesKey(); + } + + @Override public String toString() { final StringBuilder buffer = new StringBuilder("FilterReader("); buffer.append(in); Index: lucene/src/java/org/apache/lucene/index/IndexReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/IndexReader.java (revision 1214545) +++ lucene/src/java/org/apache/lucene/index/IndexReader.java (working copy) @@ -1100,13 +1100,24 @@ */ public abstract ReaderContext getTopReaderContext(); - /** Expert */ + /** Expert: Returns a key for this IndexReader, so FieldCache/CachingWrapperFilter can find + * it again. + * This key must not have equals()/hashCode() methods, so "equals" means "identical". */ public Object getCoreCacheKey() { // Don't can ensureOpen since FC calls this (to evict) // on close return this; } + /** Expert: Returns a key for this IndexReader that also includes deletions, + * so FieldCache/CachingWrapperFilter can find it again. + * This key must not have equals()/hashCode() methods, so "equals" means "identical". */ + public Object getCombinedCoreAndDeletesKey() { + // Don't can ensureOpen since FC calls this (to evict) + // on close + return this; + } + /** Returns the number of unique terms (across all fields) * in this reader. * Index: lucene/src/java/org/apache/lucene/index/SegmentReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/SegmentReader.java (revision 1214545) +++ lucene/src/java/org/apache/lucene/index/SegmentReader.java (working copy) @@ -47,7 +47,8 @@ final CloseableThreadLocal fieldsReaderLocal = new FieldsReaderLocal(); final CloseableThreadLocal termVectorsLocal = new CloseableThreadLocal(); - volatile BitVector liveDocs; + volatile BitVector liveDocs = null; + volatile Object combinedCoreAndDeletesKey; AtomicInteger liveDocsRef = null; boolean hasChanges = false; private boolean liveDocsDirty = false; @@ -159,8 +160,11 @@ if (liveDocs.size() != si.docCount) { throw new CorruptIndexException("document count mismatch: deleted docs count " + liveDocs.size() + " vs segment doc count " + si.docCount + " segment=" + si.name); } - } else + } else { assert si.getDelCount() == 0; + } + // we need a key reflecting actual deletes (if existent or not): + combinedCoreAndDeletesKey = new Object(); } @Override @@ -411,6 +415,11 @@ } @Override + public Object getCombinedCoreAndDeletesKey() { + return combinedCoreAndDeletesKey; + } + + @Override public int getTermInfosIndexDivisor() { return core.termsIndexDivisor; } @@ -465,6 +474,7 @@ core.incRef(); clone.core = core; clone.pendingDeleteCount = pendingDeleteCount; + clone.combinedCoreAndDeletesKey = combinedCoreAndDeletesKey; if (!openReadOnly && hasChanges) { // My pending changes transfer to the new reader @@ -592,6 +602,9 @@ liveDocsRef = new AtomicInteger(1); oldRef.decrementAndGet(); } + // we need a key reflecting actual deletes (if existent or not): + combinedCoreAndDeletesKey = new Object(); + // liveDocs are now dirty: liveDocsDirty = true; if (liveDocs.getAndClear(docNum)) { pendingDeleteCount++; Index: lucene/src/java/org/apache/lucene/search/CachingWrapperFilter.java =================================================================== --- lucene/src/java/org/apache/lucene/search/CachingWrapperFilter.java (revision 1214545) +++ lucene/src/java/org/apache/lucene/search/CachingWrapperFilter.java (working copy) @@ -18,17 +18,17 @@ */ import java.io.IOException; -import java.lang.ref.SoftReference; +import java.util.Collections; +import java.util.Map; import java.util.WeakHashMap; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.IndexReader.AtomicReaderContext; import org.apache.lucene.util.FixedBitSet; import org.apache.lucene.util.Bits; -import org.apache.lucene.util.WeakIdentityHashMap; /** - * Wraps another filter's result and caches it. The purpose is to allow + * Wraps another {@link Filter}'s result and caches it. The purpose is to allow * filters to simply filter, and then wrap with this class * to add caching. */ @@ -37,43 +37,33 @@ // specify the actual readers key or something similar to indicate on which // level of the readers hierarchy it should be cached. private final Filter filter; - private final FilterCache cache = new FilterCache(); + private final Map cache = Collections.synchronizedMap(new WeakHashMap()); private final boolean recacheDeletes; - private static class FilterCache { - private final WeakHashMap>> cache = - new WeakHashMap>>(); - - public synchronized DocIdSet get(IndexReader reader, Bits acceptDocs) throws IOException { - final Object coreKey = reader.getCoreCacheKey(); - WeakIdentityHashMap> innerCache = cache.get(coreKey); - if (innerCache == null) { - innerCache = new WeakIdentityHashMap>(); - cache.put(coreKey, innerCache); - } - - final SoftReference innerRef = innerCache.get(acceptDocs); - return innerRef == null ? null : innerRef.get(); - } - - public synchronized void put(IndexReader reader, Bits acceptDocs, DocIdSet value) { - cache.get(reader.getCoreCacheKey()).put(acceptDocs, new SoftReference(value)); - } - } - /** Wraps another filter's result and caches it. + * Deletions are not cached and AND'd in on the fly, see + * {@link #CachingWrapperFilter(Filter,boolean)} for an explanation. + * This constructor is recommended for often changing indexes. * @param filter Filter to cache results of + * @see #CachingWrapperFilter(Filter,boolean) */ public CachingWrapperFilter(Filter filter) { this(filter, false); } - /** Wraps another filter's result and caches it. If - * recacheDeletes is true, then new deletes (for example - * after {@link IndexReader#openIfChanged}) will be AND'd - * and cached again. + /** Wraps another filter's result and caches it. If + * {@code recacheDeletes} is {@code true}, then new deletes (for example + * after {@link IndexReader#openIfChanged}) will cause the filter + * {@link DocIdSet} to be recached. * - * @param filter Filter to cache results of + *

If your index changes seldom, it is recommended to use {@code recacheDeletes=true}, + * as recaching will only occur when the index is reopened. + * For near-real-time indexes or indexes that are often + * reopened with (e.g., {@link IndexReader#openIfChanged} is used), you should + * pass {@code recacheDeletes=false}. This will cache the filter results omitting + * deletions and will AND them in while scoring. + * @param filter Filter to cache results of + * @param recacheDeletes if deletions on the underlying index should recache */ public CachingWrapperFilter(Filter filter, boolean recacheDeletes) { this.filter = filter; @@ -84,7 +74,7 @@ * by the wrapped Filter. *

This implementation returns the given {@link DocIdSet}, if {@link DocIdSet#isCacheable} * returns true, else it copies the {@link DocIdSetIterator} into - * an {@link FixedBitSet}. + * a {@link FixedBitSet}. */ protected DocIdSet docIdSetToCache(DocIdSet docIdSet, IndexReader reader) throws IOException { if (docIdSet == null) { @@ -116,26 +106,31 @@ // Only cache if incoming acceptDocs is == live docs; // if Lucene passes in more interesting acceptDocs in - // the future we don't want to over-cache: - final boolean doCacheSubAcceptDocs = recacheDeletes && acceptDocs == reader.getLiveDocs(); + // the future (@UweSays: it already does when you chain FilteredQuery) we don't want to over-cache: + final Bits liveDocs = reader.getLiveDocs(); + final boolean doCacheAcceptDocs = (recacheDeletes && acceptDocs == liveDocs); - final Bits subAcceptDocs; - if (doCacheSubAcceptDocs) { - subAcceptDocs = acceptDocs; + final Object key; + final Bits cacheAcceptDocs; + if (doCacheAcceptDocs) { + assert acceptDocs == liveDocs; + key = reader.getCombinedCoreAndDeletesKey(); + cacheAcceptDocs = acceptDocs; } else { - subAcceptDocs = null; + key = reader.getCoreCacheKey(); + cacheAcceptDocs = null; } - DocIdSet docIdSet = cache.get(reader, subAcceptDocs); + DocIdSet docIdSet = cache.get(key); if (docIdSet != null) { hitCount++; } else { missCount++; - docIdSet = docIdSetToCache(filter.getDocIdSet(context, subAcceptDocs), reader); - cache.put(reader, subAcceptDocs, docIdSet); + docIdSet = docIdSetToCache(filter.getDocIdSet(context, cacheAcceptDocs), reader); + cache.put(key, docIdSet); } - if (doCacheSubAcceptDocs) { + if (doCacheAcceptDocs) { return docIdSet; } else { return BitsFilteredDocIdSet.wrap(docIdSet, acceptDocs); Index: lucene/src/java/org/apache/lucene/util/WeakIdentityHashMap.java =================================================================== --- lucene/src/java/org/apache/lucene/util/WeakIdentityHashMap.java (revision 1214545) +++ lucene/src/java/org/apache/lucene/util/WeakIdentityHashMap.java (working copy) @@ -1,144 +0,0 @@ -package org.apache.lucene.util; - -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import java.lang.ref.Reference; -import java.lang.ref.ReferenceQueue; -import java.lang.ref.WeakReference; -import java.util.HashMap; - -/** - * Implements a combination of {@link java.util.WeakHashMap} and - * {@link java.util.IdentityHashMap}. - * Useful for caches that need to key off of a {@code ==} comparison - * instead of a {@code .equals}. - * - *

This class is not a general-purpose {@link java.util.Map} - * implementation! It intentionally violates - * Map's general contract, which mandates the use of the equals method - * when comparing objects. This class is designed for use only in the - * rare cases wherein reference-equality semantics are required. - * - *

Note that this implementation is not synchronized. - * - *

This implementation was forked from Apache CXF - * but modified to not implement the {@link java.util.Map} interface and - * without any set/iterator views on it, as those are error-prone - * and inefficient, if not implemented carefully. Lucene's implementation also - * supports {@code null} keys, but those are never weak! - * - * @lucene.internal - */ -public class WeakIdentityHashMap { - final ReferenceQueue queue = new ReferenceQueue(); // pkg-private for inner class - private final HashMap backingStore; - - public WeakIdentityHashMap() { - backingStore = new HashMap(); - } - - public WeakIdentityHashMap(int initialCapacity) { - backingStore = new HashMap(initialCapacity); - } - - public WeakIdentityHashMap(int initialCapacity, float loadFactor) { - backingStore = new HashMap(initialCapacity, loadFactor); - } - - public void clear() { - backingStore.clear(); - reap(); - } - - public boolean containsKey(Object key) { - reap(); - return backingStore.containsKey(new IdentityWeakReference(key)); - } - - public boolean containsValue(Object value) { - reap(); - return backingStore.containsValue(value); - } - - public V get(Object key) { - reap(); - return backingStore.get(new IdentityWeakReference(key)); - } - - public V put(K key, V value) { - reap(); - return backingStore.put(new IdentityWeakReference(key), value); - } - - public boolean isEmpty() { - return size() == 0; - } - - public V remove(Object key) { - try { - reap(); - return backingStore.remove(new IdentityWeakReference(key)); - } finally { - reap(); - } - } - - public int size() { - if (backingStore.isEmpty()) - return 0; - reap(); - return backingStore.size(); - } - - private void reap() { - Reference zombie; - while ((zombie = queue.poll()) != null) { - backingStore.remove(zombie); - } - } - - final class IdentityWeakReference extends WeakReference { - private final int hash; - - IdentityWeakReference(Object obj) { - super(obj == null ? NULL : obj, queue); - hash = System.identityHashCode(obj); - } - - public int hashCode() { - return hash; - } - - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o instanceof WeakReference) { - final WeakReference ref = (WeakReference)o; - if (this.get() == ref.get()) { - return true; - } - } - return false; - } - } - - // we keep a hard reference to our NULL key, so this map supports null keys that never get GCed: - static final Object NULL = new Object(); -} - Index: lucene/src/test/org/apache/lucene/index/TestSegmentReader.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestSegmentReader.java (revision 1214545) +++ lucene/src/test/org/apache/lucene/index/TestSegmentReader.java (working copy) @@ -78,14 +78,44 @@ DocHelper.setupDoc(docToDelete); SegmentInfo info = DocHelper.writeDoc(random, dir, docToDelete); SegmentReader deleteReader = SegmentReader.getRW(info, true, IndexReader.DEFAULT_TERMS_INDEX_DIVISOR, newIOContext(random)); - assertTrue(deleteReader != null); - assertTrue(deleteReader.numDocs() == 1); + assertNotNull(deleteReader); + assertEquals(1, deleteReader.numDocs()); + final Object combKey = deleteReader.getCombinedCoreAndDeletesKey(); + final Object coreKey = deleteReader.getCoreCacheKey(); + assertNotNull(combKey); + assertNotNull(coreKey); + assertNotSame(combKey, coreKey); + + SegmentReader clone1 = (SegmentReader) deleteReader.clone(); + assertSame(coreKey, clone1.getCoreCacheKey()); + assertSame(combKey, clone1.getCombinedCoreAndDeletesKey()); + deleteReader.deleteDocument(0); + final Object newCombKey = deleteReader.getCombinedCoreAndDeletesKey(); + assertNotNull(newCombKey); + assertNotSame(combKey, newCombKey); + assertSame(coreKey, deleteReader.getCoreCacheKey()); assertFalse(deleteReader.getLiveDocs().get(0)); - assertTrue(deleteReader.hasDeletions() == true); + assertTrue(deleteReader.hasDeletions()); assertTrue(deleteReader.numDocs() == 0); + + SegmentReader clone2 = (SegmentReader) deleteReader.clone(); + assertSame(coreKey, clone2.getCoreCacheKey()); + assertSame(newCombKey, clone2.getCombinedCoreAndDeletesKey()); + assertFalse(clone2.getLiveDocs().get(0)); + assertTrue(clone2.hasDeletions()); + assertEquals(0, clone2.numDocs()); + clone2.close(); + + assertSame(coreKey, clone1.getCoreCacheKey()); + assertSame(combKey, clone1.getCombinedCoreAndDeletesKey()); + assertNull(clone1.getLiveDocs()); + assertFalse(clone1.hasDeletions()); + assertEquals(1, clone2.numDocs()); + clone1.close(); + deleteReader.close(); - } + } public void testGetFieldNameVariations() { Collection result = reader.getFieldNames(IndexReader.FieldOption.ALL); Index: lucene/src/test/org/apache/lucene/util/TestWeakIdentityHashMap.java =================================================================== --- lucene/src/test/org/apache/lucene/util/TestWeakIdentityHashMap.java (revision 1214545) +++ lucene/src/test/org/apache/lucene/util/TestWeakIdentityHashMap.java (working copy) @@ -1,78 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.lucene.util; - -import java.util.Map; -import java.util.WeakHashMap; - -public class TestWeakIdentityHashMap extends LuceneTestCase { - - public void test() { - final WeakIdentityHashMap map = - new WeakIdentityHashMap(); - // we keep strong references to the keys, - // so WeakIdentityHashMap will not forget about them: - String key1 = new String("foo"); - String key2 = new String("foo"); - String key3 = new String("foo"); - - assertNotSame(key1, key2); - assertEquals(key1, key2); - assertNotSame(key1, key3); - assertEquals(key1, key3); - assertNotSame(key2, key3); - assertEquals(key2, key3); - - map.put(key1, "bar1"); - map.put(key2, "bar2"); - map.put(null, "null"); - - assertEquals("bar1", map.get(key1)); - assertEquals("bar2", map.get(key2)); - assertEquals(null, map.get(key3)); - assertEquals("null", map.get(null)); - - assertTrue(map.containsKey(key1)); - assertTrue(map.containsKey(key2)); - assertFalse(map.containsKey(key3)); - assertTrue(map.containsKey(null)); - - assertEquals(3, map.size()); - map.remove(null); - assertEquals(2, map.size()); - map.remove(key1); - assertEquals(1, map.size()); - map.put(key1, "bar1"); - map.put(key2, "bar2"); - map.put(key3, "bar3"); - assertEquals(3, map.size()); - - // clear strong refs - key1 = key2 = key3 = null; - - // check that GC does not cause problems in reap() method: - for (int i = 0; !map.isEmpty(); i++) try { - if (i > 40) - fail("The garbage collector did not reclaim all keys after 2 seconds, failing test!"); - System.runFinalization(); - System.gc(); - Thread.currentThread().sleep(50L); - } catch (InterruptedException ie) {} - } - -}