Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9
    • Component/s: core/search
    • Labels:
      None
    • Environment:

      all

    • Lucene Fields:
      New, Patch Available

      Description

      Move ScorerDocQueue initialization from next() and skipTo() methods to the Constructor. Makes DisjunctionSumScorer a bit faster (less than 1% on my tests).

      Downside (if this is one, I cannot judge) would be throwing IOException from DisjunctionSumScorer constructors as we touch HardDisk there. I see no problem as this IOException does not propagate too far (the only modification I made is in BooleanScorer2)

      if (scorerDocQueue == null) {
      initScorerDocQueue();
      }

      Attached test is just quick & dirty rip of TestScorerPerf from standard Lucene test package. Not included as patch as I do not like it.

      All test pass, patch made on trunk revision 613923

        Activity

        Hide
        Michael McCandless added a comment -

        Thanks Eks and Paul!

        Show
        Michael McCandless added a comment - Thanks Eks and Paul!
        Hide
        Michael McCandless added a comment -

        I plan to commit shortly.

        Show
        Michael McCandless added a comment - I plan to commit shortly.
        Hide
        Eks Dev added a comment -

        test using Sun 1.4 jvm on the same hardware showed the same "a bit faster" behavior, so this is in my opinion OK to be committed.

        Show
        Eks Dev added a comment - test using Sun 1.4 jvm on the same hardware showed the same "a bit faster" behavior, so this is in my opinion OK to be committed.
        Hide
        Eks Dev added a comment -

        Well, I do not know how it behaves on earlier jvm-s and what would be the "jvm we optimize", I would not be surprised if jvm 6+ evolved optimization methods.
        These patches are just side effects of trying to get familiar with scorer family inner working in light of LUCENE-584. Boolean arithmetic on multiple skipping iterators in Scorers can hardly be beaten and can be recycled for cases like BooleanFilter... and maybe one day merged to avoid code duplication

        Anyhow, if it proves that performance on 1.4 behaves similarly, I would opt for size(), makes code slightly cleaner. If not, I would suggest to replace the only size() usage in next() with cached queueSize

        Show
        Eks Dev added a comment - Well, I do not know how it behaves on earlier jvm-s and what would be the "jvm we optimize", I would not be surprised if jvm 6+ evolved optimization methods. These patches are just side effects of trying to get familiar with scorer family inner working in light of LUCENE-584 . Boolean arithmetic on multiple skipping iterators in Scorers can hardly be beaten and can be recycled for cases like BooleanFilter... and maybe one day merged to avoid code duplication Anyhow, if it proves that performance on 1.4 behaves similarly, I would opt for size(), makes code slightly cleaner. If not, I would suggest to replace the only size() usage in next() with cached queueSize
        Hide
        Paul Elschot added a comment -

        When I wrote it, using the queueSize variable did make a minor difference in performance.
        But with the result you have, I think it's better use the size() call only.

        Show
        Paul Elschot added a comment - When I wrote it, using the queueSize variable did make a minor difference in performance. But with the result you have, I think it's better use the size() call only.
        Hide
        Eks Dev added a comment -

        Simplification of the DisjunctionSumScorer.

        • removed cached field "private int queueSize" which mirrored ScorerDocQueue.size() and
          replaced it with method call.

        It is faster with this patch, but hardly measurable (test made with attached TestScorerPerformance) 585660ms vs 586090ms. Test on WIN XP Prof. Dual Core Intel T7300 2GHz with 6.0 java -server -Xbatch

        At a moment, I have no other configurations to test it, it would be good to see what happens on jvm 1.4

        It makes sense to commit this as it simplifies (pff, ok, simpifies it a bit already complex code in DSScorer and is not slower.

        Show
        Eks Dev added a comment - Simplification of the DisjunctionSumScorer. removed cached field "private int queueSize" which mirrored ScorerDocQueue.size() and replaced it with method call. It is faster with this patch, but hardly measurable (test made with attached TestScorerPerformance) 585660ms vs 586090ms. Test on WIN XP Prof. Dual Core Intel T7300 2GHz with 6.0 java -server -Xbatch At a moment, I have no other configurations to test it, it would be good to see what happens on jvm 1.4 It makes sense to commit this as it simplifies (pff, ok, simpifies it a bit already complex code in DSScorer and is not slower.

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Eks Dev
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development