Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.1, master (7.0)
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      As reported in LUCENE-7254, DocIdSetBuilder.add has several conditionals that slow down the construction of the DocIdSet.

      1. LUCENE-7264.patch
        15 kB
        Adrien Grand

        Activity

        Hide
        jpountz Adrien Grand added a comment -

        This patch changes the DocIdSetBuilder API. add() is gone. Instead, grow() returns a new BulkAdder object that can be used to add up to the number of documents that have been passed to the grow() method. This helps save conditionals since the add method never needs to care about whether the buffer is large enough or whether to upgrade to a bitset since everything is done up-front in the grow() call.

        Show
        jpountz Adrien Grand added a comment - This patch changes the DocIdSetBuilder API. add() is gone. Instead, grow() returns a new BulkAdder object that can be used to add up to the number of documents that have been passed to the grow() method. This helps save conditionals since the add method never needs to care about whether the buffer is large enough or whether to upgrade to a bitset since everything is done up-front in the grow() call.
        Hide
        yseeley@gmail.com Yonik Seeley added a comment - - edited

        One possible downside to this change is that it changes a predictable branch (that is handled at the CPU level) into a method call... which if it's not monomorphic can be un-inlined at the point of the call and thus end up slower (method call vs predictable branch). Will be interesting to see the benchmark results.

        Show
        yseeley@gmail.com Yonik Seeley added a comment - - edited One possible downside to this change is that it changes a predictable branch (that is handled at the CPU level) into a method call... which if it's not monomorphic can be un-inlined at the point of the call and thus end up slower (method call vs predictable branch). Will be interesting to see the benchmark results.
        Hide
        jpountz Adrien Grand added a comment -

        I benchmarked it using IndexAndSearchOpenStreetMaps by temporarily using DocIdSetBuilder instead of MatchingPoints (I did not use luceneutil since its numeric range queries match too many docs). The QPS went from 33.4 (old DocIdSetBuilder.add) to 35.0 with this patch.

        In that case I think it works well since the base class is an abstract class and there are only two impls, which the JVM can efficiently optimize. (For the record, most queries of the benchmark upgrade to a bitset so both impls are used.)

        Show
        jpountz Adrien Grand added a comment - I benchmarked it using IndexAndSearchOpenStreetMaps by temporarily using DocIdSetBuilder instead of MatchingPoints (I did not use luceneutil since its numeric range queries match too many docs). The QPS went from 33.4 (old DocIdSetBuilder.add) to 35.0 with this patch. In that case I think it works well since the base class is an abstract class and there are only two impls, which the JVM can efficiently optimize . (For the record, most queries of the benchmark upgrade to a bitset so both impls are used.)
        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

        Ah, thanks for that reference... need to update my mental models

        Show
        yseeley@gmail.com Yonik Seeley added a comment - Ah, thanks for that reference... need to update my mental models
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 95c360d053a35486aa12498c6fd319aef0377bb8 in lucene-solr's branch refs/heads/branch_6x from Adrien Grand
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=95c360d ]

        LUCENE-7264: Fewer conditionals in DocIdSetBuilder.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 95c360d053a35486aa12498c6fd319aef0377bb8 in lucene-solr's branch refs/heads/branch_6x from Adrien Grand [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=95c360d ] LUCENE-7264 : Fewer conditionals in DocIdSetBuilder.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit aa81ba8642a57181a4eaa017b52d0d3c3462544b in lucene-solr's branch refs/heads/master from Adrien Grand
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=aa81ba8 ]

        LUCENE-7264: Fewer conditionals in DocIdSetBuilder.

        Show
        jira-bot ASF subversion and git services added a comment - Commit aa81ba8642a57181a4eaa017b52d0d3c3462544b in lucene-solr's branch refs/heads/master from Adrien Grand [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=aa81ba8 ] LUCENE-7264 : Fewer conditionals in DocIdSetBuilder.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 14b42fe10ba64bb4468ea8ef298e54a751f16dd3 in lucene-solr's branch refs/heads/branch_6x from Adrien Grand
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=14b42fe ]

        LUCENE-7264: Fix test bug in TestReqExclBulkScorer.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 14b42fe10ba64bb4468ea8ef298e54a751f16dd3 in lucene-solr's branch refs/heads/branch_6x from Adrien Grand [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=14b42fe ] LUCENE-7264 : Fix test bug in TestReqExclBulkScorer.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit f7b333f10583639ee3d0f2631fee41c577c60452 in lucene-solr's branch refs/heads/master from Adrien Grand
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f7b333f ]

        LUCENE-7264: Fix test bug in TestReqExclBulkScorer.

        Show
        jira-bot ASF subversion and git services added a comment - Commit f7b333f10583639ee3d0f2631fee41c577c60452 in lucene-solr's branch refs/heads/master from Adrien Grand [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f7b333f ] LUCENE-7264 : Fix test bug in TestReqExclBulkScorer.
        Hide
        hossman Hoss Man added a comment -

        Manually correcting fixVersion per Step #S5 of LUCENE-7271

        Show
        hossman Hoss Man added a comment - Manually correcting fixVersion per Step #S5 of LUCENE-7271

          People

          • Assignee:
            jpountz Adrien Grand
            Reporter:
            jpountz Adrien Grand
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development