reduce the retained memory by deducting the minimal character#61
reduce the retained memory by deducting the minimal character#61donnerpeter wants to merge 1 commit intohankcs:masterfrom
Conversation
also add a constructor with a Map to avoid single-line trie creation and bump the Java version to one supported by modern JDKs
hankcs
left a comment
hankcs
left a comment
There was a problem hiding this comment.
Thank you for the PR, nice improvement!
I left some comments in the review. Also, the save and load need to be updated to handle minChar.
|
|
||
| <properties> | ||
| <java.version>1.6</java.version> | ||
| <java.version>9</java.version> |
There was a problem hiding this comment.
Do we have any Java 9 features required by this PR, other than the test code? Some users are still running Java 6.
There was a problem hiding this comment.
No, only tests. I mostly increased the version because my current JDK refused to produce 1.6 bytecode. Are you sure such users still exist? Java 6 reached end of life quite some time ago.
| minChar = (char) Math.min(minChar, keyword.charAt(j)); | ||
| } | ||
| } | ||
| if (minChar != Character.MAX_VALUE) { |
There was a problem hiding this comment.
I'm considering an edge case where all keywords are Character.MAX_VALUE. Maybe we should start with sth like:
int minChar = Character.MAX_VALUE + 1;
A good catch, thanks! That's where it can get complicated. Should previously saved versions be loadable by the new one? If yes, I'm considering two approaches:
What would you prefer? |
2 participants
also add a constructor with a Map to avoid single-line trie creation and bump the Java version to one supported by modern JDKs