dweiss commented on code in PR #970:
URL: https://github.com/apache/lucene/pull/970#discussion_r903362582
##########
lucene/suggest/src/java/org/apache/lucene/search/suggest/document/NRTSuggesterBuilder.java:
##########
@@ -124,11 +124,15 @@ public boolean store(DataOutput output) throws
IOException {
* <p>TODO: is there a better way to make the fst built to be more
TopNSearcher friendly?
*/
private static int maxNumArcsForDedupByte(int currentNumDedupBytes) {
+ // when currentNumDedupBytes larger than 12,the result is definitely
larger than 255;
Review Comment:
```suggestion
// when currentNumDedupBytes is larger than 12, the result is definitely
larger than 255
// (avoid potential int overflow).
```
##########
lucene/suggest/src/java/org/apache/lucene/search/suggest/document/NRTSuggesterBuilder.java:
##########
@@ -124,11 +124,15 @@ public boolean store(DataOutput output) throws
IOException {
* <p>TODO: is there a better way to make the fst built to be more
TopNSearcher friendly?
*/
private static int maxNumArcsForDedupByte(int currentNumDedupBytes) {
+ // when currentNumDedupBytes larger than 12,the result is definitely
larger than 255;
Review Comment:
For currentNumDedupBytes = 11, maxArcs = (1 + 11 * 2) * 11 = 253. While this
is correct I think it'd be simpler to read the code as:
```
long maxArcs = 2 * currentNumDedupBytes + 1;
if (currentNumDedupBytes > 5) {
maxArcs *= currentNumDedupBytes;
}
return (int) Math.min(maxArcs, 255);
```
then the cap is explicit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
