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

Ensure selection of best terms is indeed O(n) #6657

Merged
merged 1 commit into from Jul 11, 2014

Conversation

alexksikes
Copy link
Contributor

Previously the size of the priority queue was wrongly set to the total number
of terms. Instead, it should be set to 'maxQueryTerms'. This makes the
selection of best terms O(n), instead of O(n*log(n)).

Jira patch: https://issues.apache.org/jira/browse/LUCENE-5795

@s1monw
Copy link
Contributor

s1monw commented Jul 2, 2014

I left a comment on the lucene issue.

@s1monw s1monw removed the review label Jul 2, 2014
@s1monw s1monw removed the review label Jul 2, 2014
int x;

Int() {
x = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Minor, but both places that use this could avoid assigning a value by always using the contents of else block if this were initialized to 0.

Int cnt = termFreqMap.get(term);
if (cnt == null) {
     cnt = new Int();
     termFreqMap.put(term, cnt);
}
cnt.x += freq;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment, I'm thinking that out of clarity we can keep the else block.

@s1monw
Copy link
Contributor

s1monw commented Jul 10, 2014

I committed this to lucene will be in 4.10

Previously the size of the priority queue was wrongly set to the total number
of terms. Instead, it should be set to 'maxQueryTerms'. This makes the
selection of best terms O(n), instead of O(n*log(n)).

Jira patch: https://issues.apache.org/jira/browse/LUCENE-5795

Closes elastic#6657
@alexksikes alexksikes merged commit af4eee5 into elastic:master Jul 11, 2014
alexksikes added a commit that referenced this pull request Jul 11, 2014
Previously the size of the priority queue was wrongly set to the total number
of terms. Instead, it should be set to 'maxQueryTerms'. This makes the
selection of best terms O(n), instead of O(n*log(n)).

Jira patch: https://issues.apache.org/jira/browse/LUCENE-5795

Closes #6657
@alexksikes alexksikes removed the review label Jul 11, 2014
@clintongormley clintongormley changed the title More Like This: ensures selection of best terms is indeed O(n) More Like This Query: Ensure selection of best terms is indeed O(n) Jul 16, 2014
@clintongormley clintongormley changed the title More Like This Query: Ensure selection of best terms is indeed O(n) Ensure selection of best terms is indeed O(n) Jun 7, 2015
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :More Like This labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v1.3.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants