[
https://issues.apache.org/jira/browse/LUCENE-1614?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12711027#action_12711027
]
Shai Erera commented on LUCENE-1614:
------------------------------------
So Mike - I've checked BS and BS2, and I don't see where does the return value
can come into play the way you describe. Perhaps I'm missing it, but here's
what I found:
* BS - advance is not supported. In nextDoc the return value is checked only to
verify if the sub scorer is done or not, therefore comparing to >= 0 seems the
better option here.
* BS2 - delegates advance and nextDoc to its counting scorer, which could have
two variants that may be affected:
** DisjunctionSumScorer - I don't see where the return value of the scorers is
even considered.
** ConjunctionScorer - both nextDoc and advance delegate the call to doNext()
which iterates over the scorers. But it only checks the return value of advance
to decide whether to continue with the iteration or not. The only thing I
changed is using advance() >= 0 instead of skipTo which returned boolean. Is
that the one you're talking about? Maybe you mean that in that scorer (only?) I
could drop the 'more' member and stop iterating when the first scorer hits
MAX_VAL, in which case it will not be less than the last doc, even if the last
one is on MAX_VAL also?
If I didn't miss anything, than that is just one scorer, which is not always
instantiated. The rest do compare the return value of nextDoc and advance to
determine what to do, exactly as they did before when the equivalent deprecated
methods returned a boolean. It seems like this change will hurt the performance
of most Scorers/DISIs more than improve the performance of BS2 in some cases
where it instantiates a ConjunctionScorer. But like I said, maybe I'm missing a
Scorer, so I'd appreciate if you can refer me to the one you had in mind.
Also, given Marvin's response above, using 0 as sentinel is no different than
using -1 in terms of "suddenly moving backwards".
I fixed the documentation of advance and reinstated the implementation of
TermScorer, so I'm ready to post an updated patch. I'd like us to resolve the
return value issue before I do that though.
> Add next() and skipTo() variants to DocIdSetIterator that return the current
> doc, instead of boolean
> ----------------------------------------------------------------------------------------------------
>
> Key: LUCENE-1614
> URL: https://issues.apache.org/jira/browse/LUCENE-1614
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Search
> Reporter: Shai Erera
> Fix For: 2.9
>
> Attachments: LUCENE-1614.patch
>
>
> See
> http://www.nabble.com/Another-possible-optimization---now-in-DocIdSetIterator-p23223319.html
> for the full discussion. The basic idea is to add variants to those two
> methods that return the current doc they are at, to save successive calls to
> doc(). If there are no more docs, return -1. A summary of what was discussed
> so far:
> # Deprecate those two methods.
> # Add nextDoc() and skipToDoc(int) that return doc, with default impl in DISI
> (calls next() and skipTo() respectively, and will be changed to abstract in
> 3.0).
> #* I actually would like to propose an alternative to the names: advance()
> and advance(int) - the first advances by one, the second advances to target.
> # Wherever these are used, do something like '(doc = advance()) >= 0' instead
> of comparing to -1 for improved performance.
> I will post a patch shortly
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: java-dev-h...@lucene.apache.org
