[ 
https://issues.apache.org/jira/browse/LUCENE-1614?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michael McCandless updated LUCENE-1614:
---------------------------------------

    Attachment: LUCENE-1614.patch


Patch is looking good!

OK I attached new patch... note that patch applies to latest
back-compat tag, so you need to run "ant download-tag" before applying
it.  Comments/questions:

  * Init'd doc = -1 in BooleanScorer

  * Can you add an "assert scorer.docID() == -1" in IndexSearcher
    right when it gets the scorer?  Then we can tease out where we
    failed to init to -1.

  * Hmm... maybe we permit DISI.docID() to also return NO_MORE_DOCS
    after the DISI is created?  EG we do this in ConjunctionScorer.
    (Or, maybe, any docID that's < the first real docID...hmmm.)

  * Made DocsWriter.applyDeletes a bit more efficient -- no need to
    check for the sentinel (it's "congruent" w/ the existing check).
    Though it did require an upcast to long, so if we do any add
    arithmetic w/ doc where doc could be NO_MORE_DOCS we must be
    careful to upcast.

  * I think we can remove the firstTime from ReqExclScorer, since
    docID() is defined to return -1 at start?

  * Made BS.coordFactors final

  * Fixed "Shai Erera vs. Mike McCandless" -> "Shai Erera
    via Mike McCandless"

  * The addition of this:
{code}
    if (docNr == -1) { // should be if nextDoc() was not called yet
      docNr = nextDoc();
    }
{code}
    in BooleanScorer2.score(Collector, int) makes me a bit nervous --
    I think one is supposed to have called nextDoc prior to calling
    this?  Also, this was added to the same method in Scorer.

  * BS2.nextDoc is still needing to check if it's supposed to call
    initCountingSumScorer?  Can we do this in ctor?

  * Actually, you did get rid of firstTime from ConjunctionScorer?
    Maybe you meant BS2?  (Oh, or maybe you meant the addition of
    "} else if (lastDoc == -1) {" in ConjunctionScorer?  So... if we
    had a way to do that sorting (in the ctor) without invoking
    nextDoc on each sub-scorer, then you could eliminate that extra
    check right?

  * Put some "nocommit" questions in the patch


> 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, LUCENE-1614.patch, LUCENE-1614.patch, 
> LUCENE-1614.patch, LUCENE-1614.patch, 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

Reply via email to