Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.8
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      If TopDocs#merge takes from and size into account it can be optimized to create a hits ScoreDoc array equal to size instead of from+size what is now the case.

      1. LUCENE-5515.patch
        8 kB
        Martijn van Groningen
      2. LUCENE-5515.patch
        7 kB
        Martijn van Groningen

        Activity

        Hide
        Martijn van Groningen added a comment -

        Patch that includes the change to only create a hits ScoreDoc array equal to size and skips over the merged hits until the "from"th element is reached.

        Show
        Martijn van Groningen added a comment - Patch that includes the change to only create a hits ScoreDoc array equal to size and skips over the merged hits until the "from"th element is reached.
        Hide
        Michael McCandless added a comment -

        +1

        It's nice that ElasticSearch is trying to use TopDocs.merge here

        Seems like this:

        if (availHitCount < start) {

        Could be <= instead? Ie, the == case is still 0 hits returned?

        Maybe move the entire while loop into the "else"? And move
        numIterOnHits into the else too.

        The javadocs state that the returned scoreDocs will have length always
        equal to size, but that's only true if there were enough hits right?
        Maybe change it to "at most size"?

        Show
        Michael McCandless added a comment - +1 It's nice that ElasticSearch is trying to use TopDocs.merge here Seems like this: if (availHitCount < start) { Could be <= instead? Ie, the == case is still 0 hits returned? Maybe move the entire while loop into the "else"? And move numIterOnHits into the else too. The javadocs state that the returned scoreDocs will have length always equal to size, but that's only true if there were enough hits right? Maybe change it to "at most size"?
        Hide
        Martijn van Groningen added a comment -

        Thanks for taking a look at it Mike, I added a new version of the patch.

        Show
        Martijn van Groningen added a comment - Thanks for taking a look at it Mike, I added a new version of the patch.
        Hide
        Michael McCandless added a comment -

        +1, thanks Martijn!

        Show
        Michael McCandless added a comment - +1, thanks Martijn!
        Hide
        ASF subversion and git services added a comment -

        Commit 1578262 from Martijn van Groningen in branch 'dev/trunk'
        [ https://svn.apache.org/r1578262 ]

        LUCENE-5515: Improved TopDocs#merge to create a merged ScoreDoc array with length of at most equal to the specified size instead of length equal to at most from + size as was before.

        Show
        ASF subversion and git services added a comment - Commit 1578262 from Martijn van Groningen in branch 'dev/trunk' [ https://svn.apache.org/r1578262 ] LUCENE-5515 : Improved TopDocs#merge to create a merged ScoreDoc array with length of at most equal to the specified size instead of length equal to at most from + size as was before.
        Hide
        ASF subversion and git services added a comment -

        Commit 1578267 from Martijn van Groningen in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1578267 ]

        Merged revision 1578262 from trunk: LUCENE-5515: Improved TopDocs#merge to create a merged ScoreDoc array with length of at most equal to the specified size instead of length equal to at most from + size as was before.

        Show
        ASF subversion and git services added a comment - Commit 1578267 from Martijn van Groningen in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1578267 ] Merged revision 1578262 from trunk: LUCENE-5515 : Improved TopDocs#merge to create a merged ScoreDoc array with length of at most equal to the specified size instead of length equal to at most from + size as was before.
        Hide
        Martijn van Groningen added a comment -

        Committed to trunk and 4x branch.

        Show
        Martijn van Groningen added a comment - Committed to trunk and 4x branch.
        Hide
        Michael McCandless added a comment -

        This seems worth mentioning in CHANGES?

        Show
        Michael McCandless added a comment - This seems worth mentioning in CHANGES?
        Hide
        ASF subversion and git services added a comment -

        Commit 1578300 from Martijn van Groningen in branch 'dev/trunk'
        [ https://svn.apache.org/r1578300 ]

        LUCENE-5515: Added missing CHANGES.TXT entry

        Show
        ASF subversion and git services added a comment - Commit 1578300 from Martijn van Groningen in branch 'dev/trunk' [ https://svn.apache.org/r1578300 ] LUCENE-5515 : Added missing CHANGES.TXT entry
        Hide
        ASF subversion and git services added a comment -

        Commit 1578305 from Martijn van Groningen in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1578305 ]

        LUCENE-5515: Added missing CHANGES.TXT entry

        Show
        ASF subversion and git services added a comment - Commit 1578305 from Martijn van Groningen in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1578305 ] LUCENE-5515 : Added missing CHANGES.TXT entry
        Hide
        ASF subversion and git services added a comment -

        Commit 1578308 from Martijn van Groningen in branch 'dev/trunk'
        [ https://svn.apache.org/r1578308 ]

        LUCENE-5515: Added author to CHANGES.txt entry

        Show
        ASF subversion and git services added a comment - Commit 1578308 from Martijn van Groningen in branch 'dev/trunk' [ https://svn.apache.org/r1578308 ] LUCENE-5515 : Added author to CHANGES.txt entry
        Hide
        Uwe Schindler added a comment -

        Close issue after release of 4.8.0

        Show
        Uwe Schindler added a comment - Close issue after release of 4.8.0

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development