Ok sleeping did help.
OK...good morning (afternoon)!
BTW, we should be aware that this means anyone using HitQueue needs to know that upon initialization it's filled with sentinel objects, and that its size() will be maxSize etc. Since HQ is package private I don't have a problem with it.
Good point – can you update HitQueue's javadocs stating this new
BTW, IndexSearch.doSearch creates the Scorer, but already receives the Collector as argument, therefore at this point it's too late to make any decisions regarding orderness of docs, no?
Urgh yeah right. So docsInOrder() should be added to Weight or Query
I guess. Maybe name it "scoresDocsInOrder()"?
Add docsInOrder to Weight (it's an interface, therefore just in 3.0)
Aside: maybe for 3.0 we should do a hard cutover of any remaining
interfaces, like Weight, Fieldable (if we don't replace it), etc. to
Remember that it may be used by IndexSearcher in two modes: (1) without a filter - BS2.score(Collector), (2) with filter - BS2.next() and skipTo().
I'd really love to find a way to make this more explicit. EG you ask
the Weight for a topScorer() vs a scorer(), or something. Clearly the
caller of .scorer() knows full well how the instance will be used (top
or not)... we keep struggling with BS/2 because this information is
not explicit now.
This would also enable BQ.scorer() to directly return a BS vs BS2,
rather than the awkward BS2 wrapping a BS internally in its
So how about adding a "topScorer()" method, that defaults to scorer()?
(Ugh, we can't do that until 3.0 since Weight is an interface).
But actually: the thing calling scoresDocsInOrder will in fact only be
calling that method if it intends to use the scorer as a toplevel
scorer (ie, it will call scorer.score(Collector), not
scorer.next/skipTo)? So BQ.scoresDocsInOrder would first check if
it's gonna defer to BS (it's a simple OR query) and then check BS's
In IS we check BQ.getAllowDocsOutOfOrder() and if true we always create out-of-order collectors. That might impact performance if there are no BQ clauses, but I assume it is not used much? And this doesn't break back-compat since that's the only way to instantiate an out-of-order Scorer today (besides creating your own).
This back compat break worries me a bit. EG maybe Solr has some
scorers that run out-of-order?
Also this is not really a clean solution: sure, it's only BQ today
that does out-of-order scoring, but I can see others doing it in the
future. I can also see making out-of-order-scoring more common and in
fact the default (again) for BQ, since it does give good performance
gains. Maybe other Query scorers should use it too.
So I think I'd prefer the "add scoresDocsOutOfOrder" to Query. Note
that this must be called on the rewritten Query.
And since they can always create the Query and only then create the Collector, if we add that info to Query they should have enough information at hand to create the proper Collector instance.
Right, adding the method to Query gives expert users the tools needed
to make their own efficient collectors.
If we do add it to Query, then I'd like to deprecate BQ's static setter and getter of that attribute and provide a docsInOrder() impl, but we need to resolve how it will know whether it will use BS or BS2.
OK, you mean make this a non-static setter/getter? I would actually
prefer to default it to "true", as well. (It's better performance).
But that should wait for 3.0 (be sure to open a "for 3.0" followon
issue for this one, too!).
BTW, even though BS does score docs out of order, it still visits docs
in order "by bucket". Meaning when visiting docs, you know the docIDs
are always greater than the last bucket's docIDs.
This gives us a source of optimization: if we hold onto the "bottom
value in the pqueue as of the last bucket", and then we can first
compare that bottom value (with no tie breaking by docID needed) and
if that competes, then we do the "true, current bottomValue with
breaking tie by docID" comparison to see if it makes it into the
queue. But I'm not sure how we'd cleanly model this "docIDs are in
order by bucket" case of the scorer, and take advantage of that during
collection. I think it'd require extending the FieldComparator API
somehow, eg "get me a BottomValueComparator instance as of right
This can also be the basis for a separate strong optimization, which
is down in the TermScorer for a BooleanScorer/2, skip a docID if its
field value is not competitive. This is a bigger change (way outside
the scope of this issue, and in fact more related to