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

hunspell: make the hash table load factor customizable #12464

Merged
merged 4 commits into from Jul 28, 2023

Conversation

donnerpeter
Copy link
Contributor

No description provided.

@donnerpeter donnerpeter requested a review from dweiss July 26, 2023 19:42
* usage.
*/
protected static double hashFactor() {
return 1.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the dumb question but how can this be increased without bytecode manipulation or source code changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ouch. Late night commits have some disadvantages :) Thanks for spotting!

@@ -390,7 +391,7 @@ private int flushGroup() throws IOException {

if (++chainLengths[hash] > 20) {
throw new RuntimeException(
"Too many collisions, please report this to dev@lucene.apache.org");
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether this message should remain as it was. Most people will use it out of the box and we can then count on their feedback if something is not right. Those who do override the default will certainly know where to look (we can report the hashFactor as part of the exception message, perhaps)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something like this? Too many collisions. Try a larger Dictionary#hashFactor. If this doesn't help, please report this to dev@lucene.apache.org?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, something like it. Maybe even provide some diagnostics?:
"Too many collisions. Try a larger Dictionary#hashFactor (now: " + hashFactor() + "). If this doesn't help, please report this to dev@lucene.apache.org"

Copy link
Contributor

@dweiss dweiss left a comment

Choose a reason for hiding this comment

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

Other than the exception message, I think it's fine.

Copy link
Contributor

@dweiss dweiss left a comment

Choose a reason for hiding this comment

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

Thanks, Peter.

@donnerpeter donnerpeter merged commit 1af68bf into apache:main Jul 28, 2023
5 checks passed
@donnerpeter donnerpeter deleted the peter/hashFactor branch July 28, 2023 16:36
donnerpeter added a commit that referenced this pull request Sep 12, 2023
* hunspell: make the hash table load factor customizable

(cherry picked from commit 1af68bf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants