Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inverse DocIdSets' heuristic to find out fast DocIdSets. #8380

Merged
merged 1 commit into from Nov 10, 2014

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Nov 7, 2014

DocIdSets.isFast(DocIdSet) has two issues:

  • it works on the DocIdSet interface while some doc sets can generate either
    slow or fast iterators depending on their options (eg. whether an OrDocIdSet is
    fast or not depends on the wrapped clauses).
  • it only works because the result of this method is only taken into account
    when a DocIdSet has non-null bits().

Here is a proposal to change this method to work on top of a DocIdSetIterator
and to use a black-list rather than a white list: slow iterators should
really be the exception rather than the rule.

*
* Do not call this method directly, use {@link DocIdSets#isFastIterator}.
*/
public abstract boolean hasFastIteration();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can not name this hasFastIterator?

I feel the name continues to encourage that such slow filters are ok. If we are planning to add support for broken iterators to DocIdSetIterator as a method (I'm honestly not sure how i feel about this), then i think the method should be something like isBroken() and should be deprecated to discourage adding more broken iterators.

@jpountz
Copy link
Contributor Author

jpountz commented Nov 7, 2014

@rmuir I renamed the method to isBroken()

@rmuir
Copy link
Contributor

rmuir commented Nov 7, 2014

+1 !

@jpountz jpountz removed the review label Nov 9, 2014
DocIdSets.isFast(DocIdSet) has two issues:
 - it works on the DocIdSet interface while some doc sets can generate either
   slow or fast sets depending on their options (eg. whether an OrDocIdSet is
   fast or not depends on the wrapped clauses).
 - it only works because the result of this method is only taken into account
   when a DocIdSet has non-null `bits()`.

This commit changes this method to work on top of a DocIdSetIterator and to use
a black-list rather than a white list: slow iterators should really be the
exception rather than the rule.

Close elastic#8380
@jpountz jpountz merged commit 1448136 into elastic:master Nov 10, 2014
@clintongormley clintongormley changed the title Internal: Inverse DocIdSets' heuristic to find out fast DocIdSets. Inverse DocIdSets' heuristic to find out fast DocIdSets. Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants