Lucene - Core
  1. Lucene - Core
  2. LUCENE-4224

Simplify MultiValuedCase in TermsIncludingScoreQuery

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 4.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      While looking at LUCENE-4214, i was trying to wrap my head around what this is doing...

      I think the code specialization in the multivalued scorer doesn't buy us any additional speed? At least according to my benchmarks?

      1. LUCENE-4224.patch
        15 kB
        Martijn van Groningen
      2. LUCENE-4224.patch
        6 kB
        Martijn van Groningen
      3. LUCENE-4224.patch
        2 kB
        Robert Muir

        Activity

        Hide
        Robert Muir added a comment -

        Here's the patch.

        Note that, both these scorers return docids out of order i think? I don't think the single-valued one should implement advance(), it should throw UOE?

        Show
        Robert Muir added a comment - Here's the patch. Note that, both these scorers return docids out of order i think? I don't think the single-valued one should implement advance(), it should throw UOE?
        Hide
        Martijn van Groningen added a comment -

        The code specialization should give a performance improvement when from terms are scattered across the terms enum of the to field (in other words when there gaps between the terms). In the case the matching terms are all next to each other this code specialization shouldn't buy any additional speed. At least this is what I'm thinking now.

        Note that, both these scorers return docids out of order i think? I don't think the single-valued one should implement advance(), it should throw UOE?

        Hmmm... I think so as well, since it iterates over the terms and per term emits doc ids. So it can never be in order.

        Show
        Martijn van Groningen added a comment - The code specialization should give a performance improvement when from terms are scattered across the terms enum of the to field (in other words when there gaps between the terms). In the case the matching terms are all next to each other this code specialization shouldn't buy any additional speed. At least this is what I'm thinking now. Note that, both these scorers return docids out of order i think? I don't think the single-valued one should implement advance(), it should throw UOE? Hmmm... I think so as well, since it iterates over the terms and per term emits doc ids. So it can never be in order.
        Hide
        Robert Muir added a comment -

        Hmmm... I think so as well, since it iterates over the terms and per term emits doc ids. So it can never be in order.

        Right, we should also ensure we throw a UOE in its Weight.scorer() if scoreDocsInOrder == false.

        Otherwise, if someone takes the Query from JoinUtil and tries to search also with a Filter, they get a UOE rather
        than something sneakier.

        Show
        Robert Muir added a comment - Hmmm... I think so as well, since it iterates over the terms and per term emits doc ids. So it can never be in order. Right, we should also ensure we throw a UOE in its Weight.scorer() if scoreDocsInOrder == false. Otherwise, if someone takes the Query from JoinUtil and tries to search also with a Filter, they get a UOE rather than something sneakier.
        Hide
        Martijn van Groningen added a comment - - edited

        Attached a new patch.

        • Added a Scorer that scores in order.
        • Existing scorer throws an UOE in the advance() method.
        Show
        Martijn van Groningen added a comment - - edited Attached a new patch. Added a Scorer that scores in order. Existing scorer throws an UOE in the advance() method.
        Hide
        Robert Muir added a comment -

        rmuir20120906-bulk-40-change

        Show
        Robert Muir added a comment - rmuir20120906-bulk-40-change
        Hide
        Martijn van Groningen added a comment -

        Updated patch. Tests pass now. Will commit shortly.

        Show
        Martijn van Groningen added a comment - Updated patch. Tests pass now. Will commit shortly.
        Hide
        Martijn van Groningen added a comment -

        Committed to trunk and branch_4x

        Show
        Martijn van Groningen added a comment - Committed to trunk and branch_4x

          People

          • Assignee:
            Martijn van Groningen
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development