You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-commits@lucene.apache.org by si...@apache.org on 2010/06/11 11:29:36 UTC

svn commit: r953621 - in /lucene/java/branches/lucene_3_0: ./ src/ src/java/org/apache/lucene/analysis/ src/java/org/apache/lucene/search/ src/test/org/apache/lucene/analysis/ src/test/org/apache/lucene/document/ src/test/org/apache/lucene/index/ src/t...

Author: simonw
Date: Fri Jun 11 09:29:35 2010
New Revision: 953621

URL: http://svn.apache.org/viewvc?rev=953621&view=rev
Log:
LUCENE-2494: use CompletionService in ParallelMultiSearcher instead of simple polling

Modified:
    lucene/java/branches/lucene_3_0/CHANGES.txt
    lucene/java/branches/lucene_3_0/src/   (props changed)
    lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/analysis/Tokenizer.java   (props changed)
    lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java   (props changed)
    lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/search/ParallelMultiSearcher.java
    lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java   (props changed)
    lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/document/TestDateTools.java   (props changed)
    lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/document/TestNumberTools.java   (props changed)
    lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/index/   (props changed)
    lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java   (props changed)
    lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/util/TestAttributeSource.java   (props changed)

Modified: lucene/java/branches/lucene_3_0/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_3_0/CHANGES.txt?rev=953621&r1=953620&r2=953621&view=diff
==============================================================================
--- lucene/java/branches/lucene_3_0/CHANGES.txt (original)
+++ lucene/java/branches/lucene_3_0/CHANGES.txt Fri Jun 11 09:29:35 2010
@@ -108,6 +108,9 @@ API Changes
 
 Optimizations
 
+* LUCENE-2494: Use CompletionService in ParallelMultiSearcher instead of
+  simple polling for resutls. (Edward Drapkin, Simon Willnauer) 
+
 * LUCENE-2135: On IndexReader.close, forcefully evict any entries from
   the FieldCache rather than waiting for the WeakHashMap to release
   the reference (Mike McCandless)

Propchange: lucene/java/branches/lucene_3_0/src/
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -0,0 +1,6 @@
+/lucene/dev/branches/branch_3x/lucene/src:941394,946651,948430
+/lucene/dev/trunk/lucene/src:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
+/lucene/java/branches/lucene_2_4/src:748824
+/lucene/java/branches/lucene_2_9/src:817269-818600,825998,829134,829881,831036,896850,909334,940987,948516
+/lucene/java/branches/lucene_2_9_back_compat_tests/src:818601-821336
+/lucene/java/trunk/src:881213,881315,881466,881819,882374,882672,882807,882888,882977,883074-883075,883554,884870,886257,886911,887347,887532,887602,887670,888247,889431-889432,889579,889866,890439,890967,890988,891189,891205,891209,891363,891377,893093,894348,897672,899627,900196,908477,908975,909360,909398,910034,910078,912407,915399,919060,920270

Propchange: lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/analysis/Tokenizer.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,5 +1,5 @@
 /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/analysis/Tokenizer.java:941394,946651,948430
-/lucene/dev/trunk/lucene/src/java/org/apache/lucene/analysis/Tokenizer.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/java/org/apache/lucene/analysis/Tokenizer.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/branches/lucene_2_4/src/java/org/apache/lucene/analysis/Tokenizer.java:748824
 /lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/analysis/Tokenizer.java:817269-818600,825998,829134,829881,831036,896850,909334,940987,948516
 /lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/lucene/analysis/Tokenizer.java:818601-821336

Propchange: lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,5 +1,5 @@
 /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java:941394,946651,948430
-/lucene/dev/trunk/lucene/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/branches/lucene_2_4/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java:748824
 /lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java:817269-818600,825998,829134,829881,831036,896850,909334,940987,948516
 /lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java:818601-821336

Modified: lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/search/ParallelMultiSearcher.java
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/search/ParallelMultiSearcher.java?rev=953621&r1=953620&r2=953621&view=diff
==============================================================================
--- lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/search/ParallelMultiSearcher.java (original)
+++ lucene/java/branches/lucene_3_0/src/java/org/apache/lucene/search/ParallelMultiSearcher.java Fri Jun 11 09:29:35 2010
@@ -18,13 +18,14 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
-import java.util.List;
-import java.util.Arrays;
-import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletionService;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
 import java.util.concurrent.Callable;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
@@ -32,7 +33,6 @@ import java.util.concurrent.locks.Reentr
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.util.NamedThreadFactory;
-import org.apache.lucene.util.PriorityQueue;
 import org.apache.lucene.util.ThreadInterruptedException;
 
 /** Implements parallel search over a set of <code>Searchables</code>.
@@ -60,18 +60,20 @@ public class ParallelMultiSearcher exten
    */
   @Override
   public int docFreq(final Term term) throws IOException {
-    @SuppressWarnings("unchecked") final Future<Integer>[] searchThreads = new Future[searchables.length];
-    for (int i = 0; i < searchables.length; i++) { // search each searchable
+    final ExecutionHelper<Integer> runner = new ExecutionHelper<Integer>(executor);
+    for(int i = 0; i < searchables.length; i++) {
       final Searchable searchable = searchables[i];
-      searchThreads[i] = executor.submit(new Callable<Integer>() {
+      runner.submit(new Callable<Integer>() {
         public Integer call() throws IOException {
           return Integer.valueOf(searchable.docFreq(term));
         }
       });
     }
-    final CountDocFreq func = new CountDocFreq();
-    foreach(func, Arrays.asList(searchThreads));
-    return func.docFreq;
+    int docFreq = 0;
+    for (Integer num : runner) {
+      docFreq += num.intValue();
+    }
+    return docFreq;
   }
 
   /**
@@ -83,20 +85,25 @@ public class ParallelMultiSearcher exten
   public TopDocs search(Weight weight, Filter filter, int nDocs) throws IOException {
     final HitQueue hq = new HitQueue(nDocs, false);
     final Lock lock = new ReentrantLock();
-    @SuppressWarnings("unchecked") final Future<TopDocs>[] searchThreads = new Future[searchables.length];
+    final ExecutionHelper<TopDocs> runner = new ExecutionHelper<TopDocs>(executor);
+    
     for (int i = 0; i < searchables.length; i++) { // search each searchable
-      searchThreads[i] = executor.submit(
+      runner.submit(
           new MultiSearcherCallableNoSort(lock, searchables[i], weight, filter, nDocs, hq, i, starts));
     }
 
-    final CountTotalHits<TopDocs> func = new CountTotalHits<TopDocs>();
-    foreach(func, Arrays.asList(searchThreads));
+    int totalHits = 0;
+    float maxScore = Float.NEGATIVE_INFINITY;
+    for (final TopDocs topDocs : runner) {
+      totalHits += topDocs.totalHits;
+      maxScore = Math.max(maxScore, topDocs.getMaxScore());
+    }
 
     final ScoreDoc[] scoreDocs = new ScoreDoc[hq.size()];
     for (int i = hq.size() - 1; i >= 0; i--) // put docs in array
       scoreDocs[i] = hq.pop();
 
-    return new TopDocs(func.totalHits, scoreDocs, func.maxScore);
+    return new TopDocs(totalHits, scoreDocs, maxScore);
   }
 
   /**
@@ -110,20 +117,22 @@ public class ParallelMultiSearcher exten
 
     final FieldDocSortedHitQueue hq = new FieldDocSortedHitQueue(nDocs);
     final Lock lock = new ReentrantLock();
-    @SuppressWarnings("unchecked") final Future<TopFieldDocs>[] searchThreads = new Future[searchables.length];
+    final ExecutionHelper<TopFieldDocs> runner = new ExecutionHelper<TopFieldDocs>(executor);
     for (int i = 0; i < searchables.length; i++) { // search each searchable
-      searchThreads[i] = executor.submit(
+      runner.submit(
           new MultiSearcherCallableWithSort(lock, searchables[i], weight, filter, nDocs, hq, sort, i, starts));
     }
-
-    final CountTotalHits<TopFieldDocs> func = new CountTotalHits<TopFieldDocs>();
-    foreach(func, Arrays.asList(searchThreads));
-
+    int totalHits = 0;
+    float maxScore = Float.NEGATIVE_INFINITY;
+    for (final TopFieldDocs topFieldDocs : runner) {
+      totalHits += topFieldDocs.totalHits;
+      maxScore = Math.max(maxScore, topFieldDocs.getMaxScore());
+    }
     final ScoreDoc[] scoreDocs = new ScoreDoc[hq.size()];
     for (int i = hq.size() - 1; i >= 0; i--) // put docs in array
       scoreDocs[i] = hq.pop();
 
-    return new TopFieldDocs(func.totalHits, scoreDocs, hq.getFields(), func.maxScore);
+    return new TopFieldDocs(totalHits, scoreDocs, hq.getFields(), maxScore);
   }
 
   /** Lower-level search API.
@@ -174,57 +183,59 @@ public class ParallelMultiSearcher exten
      searchables[i].search(weight, filter, hc);
    }
   }
+
+  @Override
+  public void close() throws IOException {
+    executor.shutdown();
+    super.close();
+  }
   
-  /*
-   * apply the function to each element of the list. This method encapsulates the logic how 
-   * to wait for concurrently executed searchables.  
+  /**
+   * A helper class that wraps a {@link CompletionService} and provides an
+   * iterable interface to the completed {@link Callable} instances.
+   * 
+   * @param <T>
+   *          the type of the {@link Callable} return value
    */
-  private <T> void foreach(Function<T> func, List<Future<T>> list) throws IOException{
-    for (Future<T> future : list) {
-      try{
-        func.apply(future.get());
+  private static final class ExecutionHelper<T> implements Iterator<T>, Iterable<T> {
+    private final CompletionService<T> service;
+    private int numTasks;
+
+    ExecutionHelper(final Executor executor) {
+      this.service = new ExecutorCompletionService<T>(executor);
+    }
+
+    public boolean hasNext() {
+      return numTasks > 0;
+    }
+
+    public void submit(Callable<T> task) {
+      this.service.submit(task);
+      ++numTasks;
+    }
+
+    public T next() {
+     if(!this.hasNext())
+       throw new NoSuchElementException();
+      try {
+        return service.take().get();
+      } catch (InterruptedException e) {
+        throw new ThreadInterruptedException(e);
       } catch (ExecutionException e) {
-        if (e.getCause() instanceof IOException)
-          throw (IOException) e.getCause();
-        throw new RuntimeException(e.getCause());
-      } catch (InterruptedException ie) {
-        throw new ThreadInterruptedException(ie);
+        throw new RuntimeException(e);
+      } finally {
+        --numTasks;
       }
     }
-  }
-
-  // Both functions could be reduced to Int as other values of TopDocs
-  // are not needed. Using sep. functions is more self documenting.
-  /**
-   * A function with one argument
-   * @param <T> the argument type
-   */
-  private static interface Function<T> {
-    abstract void apply(T t);
-  }
 
-  /**
-   * Counts the total number of hits for all {@link TopDocs} instances
-   * provided. 
-   */
-  private static final class CountTotalHits<T extends TopDocs> implements Function<T> {
-    int totalHits = 0;
-    float maxScore = Float.NEGATIVE_INFINITY;
-    
-    public void apply(T t) {
-      totalHits += t.totalHits;
-      maxScore = Math.max(maxScore, t.getMaxScore());
+    public void remove() {
+      throw new UnsupportedOperationException();
     }
-  }
-  /**
-   * Accumulates the document frequency for a term.
-   */
-  private static final class CountDocFreq implements Function<Integer>{
-    int docFreq = 0;
-    
-    public void apply(Integer t) {
-      docFreq += t.intValue();
+
+    public Iterator<T> iterator() {
+      // use the shortcut here - this is only used in a privat context
+      return this;
     }
-  }
 
+  }
 }

Propchange: lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,5 +1,5 @@
 /lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java:941394,946651,948430
-/lucene/dev/trunk/lucene/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/branches/lucene_2_4/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java:748824
 /lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java:825998,829134,829881,831036,896850,909334,940987,948516
 /lucene/java/trunk/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java:881213,881315,881466,881819,882374,882672,882807,882888,882977,883074-883075,883554,884870,886257,886911,887347,887532,887602,887670,888247,889431-889432,889579,889866,890439,890967,890988,891189,891205,891209,891363,891377,893093,894348,897672,899627,900196,908477,908975,909360,909398,910034,910078,912407,915399,919060,920270

Propchange: lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/document/TestDateTools.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,5 +1,5 @@
 /lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/document/TestDateTools.java:941394,946651,948430
-/lucene/dev/trunk/lucene/src/test/org/apache/lucene/document/TestDateTools.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/test/org/apache/lucene/document/TestDateTools.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/branches/lucene_2_4/src/test/org/apache/lucene/document/TestDateTools.java:748824
 /lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/document/TestDateTools.java:825998,829134,829881,831036,896850,909334,940987,948516
 /lucene/java/trunk/src/test/org/apache/lucene/document/TestDateTools.java:881213,881315,881466,881819,882374,882672,882807,882888,882977,883074-883075,883554,884870,886257,886911,887347,887532,887602,887670,888247,889431-889432,889579,889866,890439,890967,890988,891189,891205,891209,891363,891377,893093,894348,897672,899627,900196,908477,908975,909360,909398,910034,910078,912407,915399,919060,920270

Propchange: lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/document/TestNumberTools.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,5 +1,5 @@
 /lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/document/TestNumberTools.java:941394,946651,948430
-/lucene/dev/trunk/lucene/src/test/org/apache/lucene/document/TestNumberTools.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/test/org/apache/lucene/document/TestNumberTools.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/branches/lucene_2_4/src/test/org/apache/lucene/document/TestNumberTools.java:748824
 /lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/document/TestNumberTools.java:825998,829134,829881,831036,896850,909334,940987,948516
 /lucene/java/trunk/src/test/org/apache/lucene/document/TestNumberTools.java:881213,881315,881466,881819,882374,882672,882807,882888,882977,883074-883075,883554,884870,886257,886911,887347,887532,887602,887670,888247,889431-889432,889579,889866,890439,890967,890988,891189,891205,891209,891363,891377,893093,894348,897672,899627,900196,908477,908975,909360,909398,910034,910078,912407,915399,919060,920270

Propchange: lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/index/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,3 +1,3 @@
 /lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/index:941394,946651
-/lucene/dev/trunk/lucene/src/test/org/apache/lucene/index:929738,935522,940730,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/test/org/apache/lucene/index:929738,935522,940730,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/trunk/src/test/org/apache/lucene/index:887670,889579,889866,890439,891205,891377,920270

Propchange: lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,5 +1,5 @@
 /lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java:941394,946651,948430
-/lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/branches/lucene_2_4/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java:748824
 /lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java:825998,829134,829881,831036,896850,909334,940987,948516
 /lucene/java/trunk/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java:881213,881315,881466,881819,882374,882672,882807,882888,882977,883074-883075,883554,884870,886257,886911,887347,887532,887602,887670,888247,889431-889432,889579,889866,890439,890967,890988,891189,891205,891209,891363,891377,893093,894348,897672,899627,900196,908477,908975,909360,909398,910034,910078,912407,915399,919060,920270

Propchange: lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/util/TestAttributeSource.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jun 11 09:29:35 2010
@@ -1,5 +1,5 @@
 /lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/util/TestAttributeSource.java:941394,946651,948430
-/lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestAttributeSource.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521
+/lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestAttributeSource.java:929738,932398,935522,938989,939611,939649,940730,945420,946599,948082,948429,949288,949976,949997,950458,950613,951521,953407
 /lucene/java/branches/lucene_2_4/src/test/org/apache/lucene/util/TestAttributeSource.java:748824
 /lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/util/TestAttributeSource.java:817269-818600,825998,829134,829881,831036,896850,909334,940987,948516
 /lucene/java/branches/lucene_2_9_back_compat_tests/src/test/org/apache/lucene/util/TestAttributeSource.java:818601-821336