From 927a44881c020efd6fa79ad5633c8e96bfa716df Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 22 Apr 2016 13:08:48 +0200 Subject: [PATCH] LUCENE-7243: Removed the LeafReaderContext parameter from QueryCachingPolicy#shouldCache. --- lucene/CHANGES.txt | 3 +++ .../apache/lucene/search/LRUQueryCache.java | 4 ++-- .../lucene/search/QueryCachingPolicy.java | 18 ++++++++---------- .../UsageTrackingQueryCachingPolicy.java | 7 +------ .../lucene/search/TestIndexSearcher.java | 2 +- .../lucene/search/TestLRUQueryCache.java | 8 ++++---- .../TestUsageTrackingFilterCachingPolicy.java | 17 +---------------- .../apache/lucene/queries/TermsQueryTest.java | 16 ++-------------- .../org/apache/lucene/util/LuceneTestCase.java | 2 +- 9 files changed, 23 insertions(+), 54 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index c2561b3fd2c..18409c99685 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -40,6 +40,9 @@ API Changes * LUCENE-7150: Spatial3d gets useful APIs to create common shape queries, matching LatLonPoint. (Karl Wright via Mike McCandless) +* LUCENE-7243: Removed the LeafReaderContext parameter from + QueryCachingPolicy#shouldCache. (Adrien Grand) + Optimizations * LUCENE-7071: Reduce bytes copying in OfflineSorter, giving ~10% diff --git a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java index f4cf8dc6d15..15c0f2b508a 100644 --- a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java +++ b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java @@ -661,7 +661,7 @@ public class LRUQueryCache implements QueryCache, Accountable { DocIdSet docIdSet = get(in.getQuery(), context); if (docIdSet == null) { - if (policy.shouldCache(in.getQuery(), context)) { + if (policy.shouldCache(in.getQuery())) { docIdSet = cache(context); putIfAbsent(in.getQuery(), context, docIdSet); } else { @@ -694,7 +694,7 @@ public class LRUQueryCache implements QueryCache, Accountable { DocIdSet docIdSet = get(in.getQuery(), context); if (docIdSet == null) { - if (policy.shouldCache(in.getQuery(), context)) { + if (policy.shouldCache(in.getQuery())) { docIdSet = cache(context); putIfAbsent(in.getQuery(), context, docIdSet); } else { diff --git a/lucene/core/src/java/org/apache/lucene/search/QueryCachingPolicy.java b/lucene/core/src/java/org/apache/lucene/search/QueryCachingPolicy.java index c0f6aa82c79..fabd971b6e7 100644 --- a/lucene/core/src/java/org/apache/lucene/search/QueryCachingPolicy.java +++ b/lucene/core/src/java/org/apache/lucene/search/QueryCachingPolicy.java @@ -19,8 +19,6 @@ package org.apache.lucene.search; import java.io.IOException; -import org.apache.lucene.index.LeafReaderContext; - /** * A policy defining which filters should be cached. * @@ -40,7 +38,7 @@ public interface QueryCachingPolicy { public void onUse(Query query) {} @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { return true; } @@ -51,12 +49,12 @@ public interface QueryCachingPolicy { * in order to make decisions. */ void onUse(Query query); - /** Whether the given {@link DocIdSet} should be cached on a given segment. - * This method will be called on each leaf context to know if the filter - * should be cached on this particular leaf. The filter cache will first - * attempt to load a {@link DocIdSet} from the cache. If it is not cached - * yet and this method returns true then a cache entry will be - * generated. Otherwise an uncached set will be returned. */ - boolean shouldCache(Query query, LeafReaderContext context) throws IOException; + /** Whether the given {@link Query} is worth caching. + * This method will be called by the {@link QueryCache} to know whether to + * cache. It will first attempt to load a {@link DocIdSet} from the cache. + * If it is not cached yet and this method returns true then a + * cache entry will be generated. Otherwise an uncached scorer will be + * returned. */ + boolean shouldCache(Query query) throws IOException; } diff --git a/lucene/core/src/java/org/apache/lucene/search/UsageTrackingQueryCachingPolicy.java b/lucene/core/src/java/org/apache/lucene/search/UsageTrackingQueryCachingPolicy.java index 4eb928943e3..ab68eeb61b3 100644 --- a/lucene/core/src/java/org/apache/lucene/search/UsageTrackingQueryCachingPolicy.java +++ b/lucene/core/src/java/org/apache/lucene/search/UsageTrackingQueryCachingPolicy.java @@ -19,17 +19,12 @@ package org.apache.lucene.search; import java.io.IOException; -import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.util.FrequencyTrackingRingBuffer; /** * A {@link QueryCachingPolicy} that tracks usage statistics of recently-used * filters in order to decide on which filters are worth caching. * - * It also uses some heuristics on segments, filters and the doc id sets that - * they produce in order to cache more aggressively when the execution cost - * significantly outweighs the caching overhead. - * * @lucene.experimental */ public final class UsageTrackingQueryCachingPolicy implements QueryCachingPolicy { @@ -128,7 +123,7 @@ public final class UsageTrackingQueryCachingPolicy implements QueryCachingPolicy } @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { if (query instanceof MatchAllDocsQuery // MatchNoDocsQuery currently rewrites to a BooleanQuery, // but who knows, it might get its own Weight one day diff --git a/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java b/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java index e7cdfcdd69b..8201bf81536 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java @@ -207,7 +207,7 @@ public class TestIndexSearcher extends LuceneTestCase { assertEquals(IndexSearcher.getDefaultQueryCachingPolicy(), searcher.getQueryCachingPolicy()); QueryCachingPolicy dummyPolicy = new QueryCachingPolicy() { @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { return false; } @Override diff --git a/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java b/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java index 2552b04dffc..63ccdd879a3 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java @@ -65,7 +65,7 @@ public class TestLRUQueryCache extends LuceneTestCase { public void onUse(Query query) {} @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { return false; } @@ -455,7 +455,7 @@ public class TestLRUQueryCache extends LuceneTestCase { final QueryCachingPolicy countingPolicy = new QueryCachingPolicy() { @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { return random().nextBoolean(); } @@ -762,7 +762,7 @@ public class TestLRUQueryCache extends LuceneTestCase { final QueryCachingPolicy policy = new QueryCachingPolicy() { @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { assertEquals(expectedCacheKey, query); return true; } @@ -1080,7 +1080,7 @@ public class TestLRUQueryCache extends LuceneTestCase { } @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { return true; } } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestUsageTrackingFilterCachingPolicy.java b/lucene/core/src/test/org/apache/lucene/search/TestUsageTrackingFilterCachingPolicy.java index c656b855930..29ed22fb63b 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestUsageTrackingFilterCachingPolicy.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestUsageTrackingFilterCachingPolicy.java @@ -16,15 +16,8 @@ */ package org.apache.lucene.search; -import org.apache.lucene.document.Document; import org.apache.lucene.document.IntPoint; -import org.apache.lucene.index.DirectoryReader; -import org.apache.lucene.index.IndexReader; -import org.apache.lucene.index.IndexWriter; -import org.apache.lucene.index.IndexWriterConfig; -import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.Term; -import org.apache.lucene.store.Directory; import org.apache.lucene.util.LuceneTestCase; public class TestUsageTrackingFilterCachingPolicy extends LuceneTestCase { @@ -41,15 +34,7 @@ public class TestUsageTrackingFilterCachingPolicy extends LuceneTestCase { for (int i = 0; i < 1000; ++i) { policy.onUse(q); } - Directory dir = newDirectory(); - IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null)); - w.addDocument(new Document()); - IndexReader r = DirectoryReader.open(w); - assertFalse(policy.shouldCache(q, getOnlyLeafReader(r).getContext())); - - r.close(); - w.close(); - dir.close(); + assertFalse(policy.shouldCache(q)); } } diff --git a/lucene/queries/src/test/org/apache/lucene/queries/TermsQueryTest.java b/lucene/queries/src/test/org/apache/lucene/queries/TermsQueryTest.java index a87e45d23f0..71eeef45a54 100644 --- a/lucene/queries/src/test/org/apache/lucene/queries/TermsQueryTest.java +++ b/lucene/queries/src/test/org/apache/lucene/queries/TermsQueryTest.java @@ -330,24 +330,12 @@ public class TermsQueryTest extends LuceneTestCase { } public void testIsConsideredCostlyByQueryCache() throws IOException { - Directory dir = newDirectory(); - IndexWriterConfig iwc = newIndexWriterConfig(); - IndexWriter w = new IndexWriter(dir, iwc); - Document doc = new Document(); - for (int i = 0; i < 10000; ++i) { - w.addDocument(doc); - } - w.forceMerge(1); - DirectoryReader reader = DirectoryReader.open(w); - w.close(); TermsQuery query = new TermsQuery(new Term("foo", "bar"), new Term("foo", "baz")); UsageTrackingQueryCachingPolicy policy = new UsageTrackingQueryCachingPolicy(); - assertFalse(policy.shouldCache(query, getOnlyLeafReader(reader).getContext())); + assertFalse(policy.shouldCache(query)); policy.onUse(query); policy.onUse(query); // cached after two uses - assertTrue(policy.shouldCache(query, getOnlyLeafReader(reader).getContext())); - reader.close(); - dir.close(); + assertTrue(policy.shouldCache(query)); } } diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java index 70ed86d099a..52aca7e8f52 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java @@ -476,7 +476,7 @@ public abstract class LuceneTestCase extends Assert { public void onUse(Query query) {} @Override - public boolean shouldCache(Query query, LeafReaderContext context) throws IOException { + public boolean shouldCache(Query query) throws IOException { return random().nextBoolean(); }