You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "DivyanshIITB (via GitHub)" <gi...@apache.org> on 2025/03/19 14:30:40 UTC
[PR] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
DivyanshIITB opened a new pull request, #14373:
URL: https://github.com/apache/lucene/pull/14373
This PR optimizes ParallelLeafReader to avoid redundant term vector fetching.
- Replaces per-field term vector fetching with a single call per reader.
- Reduces complexity from O(n^2) to O(n).
- Improves performance when handling large numbers of fields.
- Verified via existing tests.
Closes #7926
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "vigyasharma (via GitHub)" <gi...@apache.org>.
vigyasharma commented on code in PR #14373:
URL: https://github.com/apache/lucene/pull/14373#discussion_r2008971160
##########
lucene/core/src/java/org/apache/lucene/index/ParallelLeafReader.java:
##########
@@ -348,15 +348,24 @@ public void prefetch(int docID) throws IOException {
@Override
public Fields get(int docID) throws IOException {
ParallelFields fields = null;
- for (Map.Entry<String, LeafReader> ent : tvFieldToReader.entrySet()) {
- String fieldName = ent.getKey();
- TermVectors termVectors = readerToTermVectors.get(ent.getValue());
- Terms vector = termVectors.get(docID, fieldName);
- if (vector != null) {
- if (fields == null) {
- fields = new ParallelFields();
- }
- fields.addField(fieldName, vector);
+
+ // Step 2: Fetch all term vectors once per reader
+ for (Map.Entry<LeafReader, TermVectors> entry : readerToTermVectors.entrySet()) {
+ TermVectors termVectors = entry.getValue();
+ Fields docFields = termVectors.get(docID); // Fetch all fields at once
+
+ if (docFields != null) {
+ if (fields == null) {
+ fields = new ParallelFields();
+ }
+
+ // Step 3: Aggregate only required fields
+ for (String fieldName : docFields) {
+ Terms vector = docFields.terms(fieldName);
+ if (vector != null) {
Review Comment:
> `docFields.terms(fieldName)` can be null if the field exists in `docFields` but does not have term vectors stored.
Okay. I thought `termVectors.get(docID)` only returns Fields that have terms.
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "vigyasharma (via GitHub)" <gi...@apache.org>.
vigyasharma merged PR #14373:
URL: https://github.com/apache/lucene/pull/14373
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "vigyasharma (via GitHub)" <gi...@apache.org>.
vigyasharma commented on PR #14373:
URL: https://github.com/apache/lucene/pull/14373#issuecomment-2747063076
Changes merged. Thanks @DivyanshIITB !
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "DivyanshIITB (via GitHub)" <gi...@apache.org>.
DivyanshIITB commented on PR #14373:
URL: https://github.com/apache/lucene/pull/14373#issuecomment-2745182601
> Changes look good to me. Can you run `./gradlew tidy` to fix formatting issues, and add a changes entry before we merge this?
I successfully ran `./gradlew tidy` and the built was successful.
Also added the changes entry in `CHANGES.txt`
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "DivyanshIITB (via GitHub)" <gi...@apache.org>.
DivyanshIITB commented on PR #14373:
URL: https://github.com/apache/lucene/pull/14373#issuecomment-2741117231
Just a gentle reminder
@vigyasharma
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "DivyanshIITB (via GitHub)" <gi...@apache.org>.
DivyanshIITB commented on code in PR #14373:
URL: https://github.com/apache/lucene/pull/14373#discussion_r2009127681
##########
lucene/CHANGES.txt:
##########
@@ -35,6 +35,10 @@ Optimizations
---------------------
* GITHUB#14011: Reduce allocation rate in HNSW concurrent merge. (Viliam Durina)
* GITHUB#14022: Optimize DFS marking of connected components in HNSW by reducing stack depth, improving performance and reducing allocations. (Viswanath Kuchibhotla)
+* GITHUB#14373: Optimized `ParallelLeafReader` to improve term vector fetching efficiency.
+- Fetches all term vectors once per reader instead of per field.
+- Reduces complexity from **O(n²) to O(n)**.
+- Enhances performance for documents with many fields. (Divyansh Agrawal)
Review Comment:
I have modified `CHANGES.txt` as you said.
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "DivyanshIITB (via GitHub)" <gi...@apache.org>.
DivyanshIITB commented on PR #14373:
URL: https://github.com/apache/lucene/pull/14373#issuecomment-2746245859
> > I successfully ran `./gradlew tidy` and the built was successful.
>
> Github build is still failing on spotless (formatting). `tidy` will change and reformat offending files for you, you need to commit and push those changes.
Thanks for the review! 🙌 I have run `./gradlew tidy` and pushed the formatting fixes. Let me know if there's anything else needed.
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "vigyasharma (via GitHub)" <gi...@apache.org>.
vigyasharma commented on code in PR #14373:
URL: https://github.com/apache/lucene/pull/14373#discussion_r2008678599
##########
lucene/core/src/java/org/apache/lucene/index/ParallelLeafReader.java:
##########
@@ -348,15 +348,24 @@ public void prefetch(int docID) throws IOException {
@Override
public Fields get(int docID) throws IOException {
ParallelFields fields = null;
- for (Map.Entry<String, LeafReader> ent : tvFieldToReader.entrySet()) {
- String fieldName = ent.getKey();
- TermVectors termVectors = readerToTermVectors.get(ent.getValue());
- Terms vector = termVectors.get(docID, fieldName);
- if (vector != null) {
- if (fields == null) {
- fields = new ParallelFields();
- }
- fields.addField(fieldName, vector);
+
+ // Step 2: Fetch all term vectors once per reader
+ for (Map.Entry<LeafReader, TermVectors> entry : readerToTermVectors.entrySet()) {
+ TermVectors termVectors = entry.getValue();
+ Fields docFields = termVectors.get(docID); // Fetch all fields at once
+
+ if (docFields != null) {
+ if (fields == null) {
+ fields = new ParallelFields();
+ }
+
+ // Step 3: Aggregate only required fields
+ for (String fieldName : docFields) {
+ Terms vector = docFields.terms(fieldName);
+ if (vector != null) {
Review Comment:
When would this be null? Since we're going through fields returned by `termVectors.get(docId)`, the field should exist and have terms.
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "vigyasharma (via GitHub)" <gi...@apache.org>.
vigyasharma commented on PR #14373:
URL: https://github.com/apache/lucene/pull/14373#issuecomment-2745914617
> I successfully ran `./gradlew tidy` and the built was successful.
Github build is still failing on spotless (formatting). `tidy` will change and reformat offending files for you, you need to commit and push those changes.
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "vigyasharma (via GitHub)" <gi...@apache.org>.
vigyasharma commented on code in PR #14373:
URL: https://github.com/apache/lucene/pull/14373#discussion_r2008971507
##########
lucene/CHANGES.txt:
##########
@@ -35,6 +35,10 @@ Optimizations
---------------------
* GITHUB#14011: Reduce allocation rate in HNSW concurrent merge. (Viliam Durina)
* GITHUB#14022: Optimize DFS marking of connected components in HNSW by reducing stack depth, improving performance and reducing allocations. (Viswanath Kuchibhotla)
+* GITHUB#14373: Optimized `ParallelLeafReader` to improve term vector fetching efficiency.
+- Fetches all term vectors once per reader instead of per field.
+- Reduces complexity from **O(n²) to O(n)**.
+- Enhances performance for documents with many fields. (Divyansh Agrawal)
Review Comment:
We generally keep a single bullet per changes entry. Details are already available in the pull request that the entry points too.
--
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] Optimize ParallelLeafReader to improve term vector fetching efficienc [lucene]
Posted by "DivyanshIITB (via GitHub)" <gi...@apache.org>.
DivyanshIITB commented on code in PR #14373:
URL: https://github.com/apache/lucene/pull/14373#discussion_r2008721150
##########
lucene/core/src/java/org/apache/lucene/index/ParallelLeafReader.java:
##########
@@ -348,15 +348,24 @@ public void prefetch(int docID) throws IOException {
@Override
public Fields get(int docID) throws IOException {
ParallelFields fields = null;
- for (Map.Entry<String, LeafReader> ent : tvFieldToReader.entrySet()) {
- String fieldName = ent.getKey();
- TermVectors termVectors = readerToTermVectors.get(ent.getValue());
- Terms vector = termVectors.get(docID, fieldName);
- if (vector != null) {
- if (fields == null) {
- fields = new ParallelFields();
- }
- fields.addField(fieldName, vector);
+
+ // Step 2: Fetch all term vectors once per reader
+ for (Map.Entry<LeafReader, TermVectors> entry : readerToTermVectors.entrySet()) {
+ TermVectors termVectors = entry.getValue();
+ Fields docFields = termVectors.get(docID); // Fetch all fields at once
+
+ if (docFields != null) {
+ if (fields == null) {
+ fields = new ParallelFields();
+ }
+
+ // Step 3: Aggregate only required fields
+ for (String fieldName : docFields) {
+ Terms vector = docFields.terms(fieldName);
+ if (vector != null) {
Review Comment:
`docFields.terms(fieldName)` can be null if the field exists in `docFields` but does not have term vectors stored. This check ensures we only add fields that actually have term vectors
--
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