Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-1575

Refactoring Lucene collectors (HitCollector and extensions)

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9
    • Component/s: core/search
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      This issue is a result of a recent discussion we've had on the mailing list. You can read the thread here.

      We have agreed to do the following refactoring:

      • Rename MultiReaderHitCollector to Collector, with the purpose that it will be the base class for all Collector implementations.
      • Deprecate HitCollector in favor of the new Collector.
      • Introduce new methods in IndexSearcher that accept Collector, and deprecate those that accept HitCollector.
        • Create a final class HitCollectorWrapper, and use it in the deprecated methods in IndexSearcher, wrapping the given HitCollector.
        • HitCollectorWrapper will be marked deprecated, so we can remove it in 3.0, when we remove HitCollector.
        • It will remove any instanceof checks that currently exist in IndexSearcher code.
      • Create a new (abstract) TopDocsCollector, which will:
        • Leave collect and setNextReader unimplemented.
        • Introduce protected members PriorityQueue and totalHits.
        • Introduce a single protected constructor which accepts a PriorityQueue.
        • Implement topDocs() and getTotalHits() using the PQ and totalHits members. These can be used as-are by extending classes, as well as be overridden.
        • Introduce a new topDocs(start, howMany) method which will be used a convenience method when implementing a search application which allows paging through search results. It will also attempt to improve the memory allocation, by allocating a ScoreDoc[] of the requested size only.
      • Change TopScoreDocCollector to extend TopDocsCollector, use the topDocs() and getTotalHits() implementations as they are from TopDocsCollector. The class will also be made final.
      • Change TopFieldCollector to extend TopDocsCollector, and make the class final. Also implement topDocs(start, howMany).
      • Change TopFieldDocCollector (deprecated) to extend TopDocsCollector, instead of TopScoreDocCollector. Implement topDocs(start, howMany)
      • Review other places where HitCollector is used, such as in Scorer, deprecate those places and use Collector instead.

      Additionally, the following proposal was made w.r.t. decoupling score from collect():

      • Change collect to accecpt only a doc Id (unbased).
      • Introduce a setScorer(Scorer) method.
      • If during collect the implementation needs the score, it can call scorer.score().
        If we do this, then we need to review all places in the code where collect(doc, score) is called, and assert whether Scorer can be passed. Also this raises few questions:
      • What if during collect() Scorer is null? (i.e., not set) - is it even possible?
      • I noticed that many (if not all) of the collect() implementations discard the document if its score is not greater than 0. Doesn't it mean that score is needed in collect() always?

      Open issues:

      • The name for Collector
      • TopDocsCollector was mentioned on the thread as TopResultsCollector, but that was when we thought to call Colletor ResultsColletor. Since we decided (so far) on Collector, I think TopDocsCollector makes sense, because of its TopDocs output.
      • Decoupling score from collect().

      I will post a patch a bit later, as this is expected to be a very large patch. I will split it into 2: (1) code patch (2) test cases (moving to use Collector instead of HitCollector, as well as testing the new topDocs(start, howMany) method.
      There might be even a 3rd patch which handles the setScorer thing in Collector (maybe even a different issue?)

        Attachments

        1. sortCollate5.py
          1 kB
          Michael McCandless
        2. sortBench5.py
          6 kB
          Michael McCandless
        3. PerfTest.java
          2 kB
          Michael McCandless
        4. LUCENE-1575.patch
          132 kB
          Shai Erera
        5. LUCENE-1575.patch
          163 kB
          Michael McCandless
        6. LUCENE-1575.patch
          200 kB
          Michael McCandless
        7. LUCENE-1575.patch
          213 kB
          Michael McCandless
        8. LUCENE-1575.patch
          215 kB
          Michael McCandless
        9. LUCENE-1575.9.patch
          226 kB
          Shai Erera
        10. LUCENE-1575.9.patch
          234 kB
          Shai Erera
        11. LUCENE-1575.8.patch
          224 kB
          Shai Erera
        12. LUCENE-1575.7.patch
          194 kB
          Shai Erera
        13. LUCENE-1575.6.patch
          161 kB
          Shai Erera
        14. LUCENE-1575.5.patch
          149 kB
          Shai Erera
        15. LUCENE-1575.4.patch
          140 kB
          Shai Erera
        16. LUCENE-1575.3.patch
          133 kB
          Shai Erera
        17. LUCENE-1575.2.patch
          132 kB
          Shai Erera
        18. LUCENE-1575.1.patch
          132 kB
          Shai Erera

          Activity

            People

            • Assignee:
              mikemccand Michael McCandless
              Reporter:
              shaie Shai Erera
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: