torstenkuhnhenne opened a new issue, #16874:
URL: https://github.com/apache/lucene/issues/16874

   ### Description
   
   `PerFieldMergeState.FilterFieldsProducer#terms(String)` checks whether the 
requested field is part of the filtered field set with `List#contains`:
   
   ``` 
   private final List<String> filtered;
   ...
   public Terms terms(String field) {
       if (!filtered.contains(field)) {
   ...
   ``` 
   
   During a merge, `PerFieldPostingsFormat.FieldsWriter#merge` creates one 
`FilterFieldsProducer` per postings format group, and the terms writer then 
calls `terms(field)` once for every field of that group. So each merge does 
O(n²) string comparisons in the number of fields.
   
   This is the same pattern that LUCENE-8558 (#9604) fixed in 
`PerFieldMergeState.FilterFieldInfos` in 2018 (`filterFields.contains(...)` was 
replaced with a `HashSet` lookup). `FilterFieldsProducer` in the same file 
still uses the `List`.
   
     ### Impact
   
   We hit this in a Solr 10.0.0 / Lucene 10.3.2 installation whose schema uses 
dynamic fields heavily. While a bulk update was being processed: Solr used 6–10 
cores, and 59 % of its CPU samples (JFR, `settings=profile`) were in `Lucene 
Merge Thread`s.
   
    **38–40 % of all Solr CPU samples** had this stack:
   
   ```
           java.util.ArrayList.indexOfRange
           java.util.ArrayList.indexOf
           java.util.ArrayList.contains
           
org.apache.lucene.codecs.perfield.PerFieldMergeState$FilterFieldsProducer.terms
           org.apache.lucene.index.MultiFields.terms
           org.apache.lucene.index.MappedMultiFields.terms
           
org.apache.lucene.codecs.lucene103.blocktree.Lucene103BlockTreeTermsWriter.write
           org.apache.lucene.codecs.FieldsConsumer.merge
           
org.apache.lucene.codecs.perfield.PerFieldPostingsFormat$FieldsWriter.merge
           org.apache.lucene.index.SegmentMerger.mergeTerms
           ...
   ```
   
   We applied this change to Lucene 10.3.2 (only `PerFieldMergeState` 
recompiled, the rest of the jar unchanged) and repeated the same bulk update in 
the same Solr 10.0.0 setup, alternating original and patched jar:
   
     | | original 10.3.2 (2 runs) | patched (2 runs) |
     |---|---|---|
     | Solr CPU samples during the bulk update (JFR) | 113k / 104k | 61k / 66k 
(−41…46 %) |
     | `FilterFieldsProducer.terms` share of CPU | 38–39 % | 1.9 % |
     | Merge threads share of CPU | 57–59 % | 35.5 % |
     | `SegmentMerger.mergeTerms` share of CPU | 47–48 % | 17 % |
     | Solr cores busy while indexing | ~6–7 | ~4 |
   
   Many fields is admittedly an unusual setup (see also #8699), but the fix is 
trivial and removes a quadratic term from every merge.
   
     ### Change
   
   Keep the `List` so that `iterator()` and `size()` behave as before, and add 
a `Set` for the membership check, the same way as `FilterFieldInfos` already 
does it (`filteredNames`). Or we can use a `LinkedHashSet` to keep the order.
   
   I am willing to provide a PR for this if you guide me which way to go.
   
   
   ### Version and environment details
   
   * Lucene 10.3.2 integrated in Solr 10
   * Linux
   * Java 25 - Eclipse Adoptium


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to