You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "neoremind (via GitHub)" <gi...@apache.org> on 2026/08/11 15:36:02 UTC
[PR] Clean up BytesRefHash [lucene]
neoremind opened a new pull request, #16499:
URL: https://github.com/apache/lucene/pull/16499
* Remove the lastCount field as it is not in use, the `!= -1` guard in `shrink()` has been not applicable
* Remove close(): it has no callers, also `Map` data structure typically does not provide such operation
* Fix get() javadoc: valid ids are non-negative, not positive (0 is a valid id)
* Fix the rehash() comment: the shrink threshold is 25% occupancy, not 20%
* Remove a stale TODO comment in DirectBytesStartArray
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Clean up BytesRefHash [lucene]
Posted by "neoremind (via GitHub)" <gi...@apache.org>.
neoremind commented on PR #16499:
URL: https://github.com/apache/lucene/pull/16499#issuecomment-5265640826
> I know this is over 10 years old... but somewhat related (for a follow-up) is this old issue - #6916
Thank you @dweiss and @uschindler for pointing me to the historical conversation! I have shared my findings and two cents on https://github.com/apache/lucene/issues/11608 and https://github.com/apache/lucene/issues/6916.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Clean up BytesRefHash [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #16499:
URL: https://github.com/apache/lucene/pull/16499#issuecomment-5258633179
I know this is over 10 years old... but somewhat related (for a follow-up) is this old issue - #6916
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Clean up BytesRefHash [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on code in PR #16499:
URL: https://github.com/apache/lucene/pull/16499#discussion_r3759552087
##########
lucene/core/src/java/org/apache/lucene/util/BytesRefHash.java:
##########
@@ -428,7 +418,7 @@ public int addByPoolOffset(int offset) {
}
/**
- * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 20%} occupied).
+ * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 25%} occupied).
Review Comment:
Do we ever shrink the table while it still has elements? I think we only do that in `clear()`? So maybe remove the `and too large` in this javadoc? We only ever call `rehash` on growing?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Clean up BytesRefHash [lucene]
Posted by "neoremind (via GitHub)" <gi...@apache.org>.
neoremind commented on code in PR #16499:
URL: https://github.com/apache/lucene/pull/16499#discussion_r3765649956
##########
lucene/core/src/java/org/apache/lucene/util/BytesRefHash.java:
##########
@@ -428,7 +418,7 @@ public int addByPoolOffset(int offset) {
}
/**
- * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 20%} occupied).
+ * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 25%} occupied).
Review Comment:
Thank you Uwe! Issue created https://github.com/apache/lucene/issues/16507
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Clean up BytesRefHash [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler merged PR #16499:
URL: https://github.com/apache/lucene/pull/16499
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Clean up BytesRefHash [lucene]
Posted by "neoremind (via GitHub)" <gi...@apache.org>.
neoremind commented on code in PR #16499:
URL: https://github.com/apache/lucene/pull/16499#discussion_r3764879491
##########
lucene/core/src/java/org/apache/lucene/util/BytesRefHash.java:
##########
@@ -428,7 +418,7 @@ public int addByPoolOffset(int offset) {
}
/**
- * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 20%} occupied).
+ * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 25%} occupied).
Review Comment:
Yes, `shrink()` is only called inside `clear()`. And `clear()` is only triggered when `storeTermVectors` is enabled on a field. My read is that the term vectors [path](https://github.com/apache/lucene/blob/d0ba760f5f08b087f5e83ccefd0f5c771ed0487f/lucene/core/src/java/org/apache/lucene/index/TermVectorsConsumer.java#L119) has a per-document lifecycle, a dedicated `BytesRefHash` grows and shrinks as each document is indexed. This is unlike regular inverted index path for doc id, freq, and prox where `clear()` is never called, when segment flushes, we recreate the indexing needed data structures from scratch for the next segment, so the previous `BytesRefHash` just gets GCed away.
I debugged this locally. For example, if the previous doc's term-vector-enabled field contains 40 unique terms, the `ids[]` grows to 128. Then `clear()` is called, no size change since 40 is between 25%-50% of 128. But if the next doc's term-vector-enabled field only has 14 terms, the `ids[]` shrinks to 32 on that doc's `clear()` call to keep `BytesRefHash` sizing best-fit to the current doc's term count, always between 25%-50%. I think such always-on-resizing does create many short-lived `int[]` that may press GC, maybe a separate performance optimization thread is needed.
<img width="1329" height="618" alt="Screenshot 2026-08-12 at 11 33 52 AM" src="https://github.com/user-attachments/assets/836baec4-b944-4e0b-8e54-803ea7415dd7" />
Revised the comment. To be precise, `rehash()` is called when count reaches 50% of hashSize, never goes over.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Clean up BytesRefHash [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16499:
URL: https://github.com/apache/lucene/pull/16499#discussion_r3765044828
##########
lucene/core/src/java/org/apache/lucene/util/BytesRefHash.java:
##########
@@ -428,7 +418,7 @@ public int addByPoolOffset(int offset) {
}
/**
- * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 20%} occupied).
+ * Called when hash is too small ({@code > 50%} occupied) or too large ({@code < 25%} occupied).
Review Comment:
OK, thanks, looks like a good followup issue (and also the one referred to by @dweiss https://github.com/apache/lucene/issues/6916). I will now merge this once CI is happy!
Uwe
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org