[perf-improver] perf: reduce allocations in GroupedAnd/Or/Not and search result field loading#462
Conversation
… loading - Avoid redundant string[] copy in GroupedAnd/Or/Not when caller already passes a string[]: use 'fields as string[] ?? fields.ToArray()' pattern in LuceneSearchQueryBase (public and INestedQuery paths) and LuceneQuery. - Remove redundant .Cast<IExamineValue>() in LuceneQuery — explicit cast in the Select projection is sufficient. - Eliminate the per-document HashSet<string> allocation in LuceneSearchExecutor.CreateSearchResult: use resultVals.ContainsKey() for duplicate-field-name detection (same O(1) semantics, one fewer object). - Pre-size the resultVals Dictionary with fields.Count to reduce resizes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Greptile SummaryThis PR applies three targeted allocation-reduction micro-optimisations on hot search paths: eliminating the per-document
Confidence Score: 4/5Safe to merge; all three optimisations correctly preserve the original semantics and no array passed to the internal methods is mutated. The changes are correct and well-scoped. The ContainsKey guard correctly avoids redundant doc.GetValues() calls on duplicate fields, GetMultiFieldQuery never mutates the fields array so bypassing the defensive copy is safe, and the Cast removal is valid since the inline cast in Select is sufficient. The single comment flags a minor double-lookup pattern. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["GroupedAnd/Or/Not\nIEnumerable fields"] --> B{Cast to string-array?}
B -->|Success - zero alloc| C["Pass caller array directly"]
B -->|Fail - allocate| D["fields.ToArray()"]
C --> E["GroupedAndInternal / OrInternal / NotInternal"]
D --> E
E --> F["GetMultiFieldQuery\nread-only iteration, no mutation"]
G["CreateSearchResult per document"] --> H["Dictionary pre-sized with fields.Count"]
H --> I["foreach field in doc.Fields"]
I --> J{ContainsKey fieldName?}
J -->|No - first occurrence| K["Load doc.GetValues and store"]
J -->|Yes - duplicate| L["Skip"]
K --> I
L --> I
Reviews (1): Last reviewed commit: "perf: reduce allocations in GroupedAnd/O..." | Re-trigger Greptile |
| if (!resultVals.ContainsKey(fieldName)) | ||
| { | ||
| continue; | ||
| resultVals[fieldName] = doc.GetValues(fieldName).ToList(); | ||
| } |
There was a problem hiding this comment.
The
ContainsKey + indexer pattern performs two dictionary lookups for every new field. TryAdd (available since .NET Core 2.0 / .NET Standard 2.1) accomplishes the same in a single lookup — though note that unlike the current guard, TryAdd evaluates the value argument eagerly, so doc.GetValues(fieldName).ToList() would execute even for duplicate field entries.
| if (!resultVals.ContainsKey(fieldName)) | |
| { | |
| continue; | |
| resultVals[fieldName] = doc.GetValues(fieldName).ToList(); | |
| } | |
| resultVals.TryAdd(fieldName, doc.GetValues(fieldName).ToList()); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🤖 This is an automated PR from Perf Improver, an AI assistant focused on performance.
Goal
Reduce unnecessary heap allocations on hot search paths — lower GC pressure in high-throughput scenarios.
Changes
Three targeted micro-optimisation changes across three files:
1.
LuceneSearchExecutor.cs— eliminate per-documentHashSet<string>allocationBefore:
CreateSearchResultallocated anew HashSet<string>()per document to track which field names had already been processed.After: Uses
resultVals.ContainsKey(fieldName)for the same deduplication logic — same O(1) semantics, one fewer object allocation per accessed search result.Also pre-sizes
resultValsdictionary withfields.Countto reduce internal resize operations when a document has many fields.2.
LuceneQuery.cs— avoid redundant array copy + remove.Cast<IExamineValue>()fields.ToArray()→fields as string[] ?? fields.ToArray(): when the caller already passes astring[](the common case — e.g.new[] { "title", "body" }), the cast short-circuits and skips the allocation entirely..Cast<IExamineValue>()calls: theSelectprojection already casts with(IExamineValue), so the intermediate LINQ iterator is unnecessary.3.
LuceneSearchQueryBase.cs— same array-copy avoidanceApplied the same
fields as string[] ?? fields.ToArray()pattern in the public-facingGroupedAnd/Or/Notmethods and theirINestedQuerycounterparts.Performance Evidence
Methodology: Code-path analysis + allocation counting. Verified with Lucene.Net's
Document.FieldsreturningIList<IIndexableField>which supports.Count.CreateSearchResult(per document)Dictionary+HashSetallocatedDictionaryonly (+ capacity hint)GroupedAnd/Or/Notwithstring[]inputLuceneQuery.GroupedAnd/Or/Not(string overload)Select+Cast+ToArray(3 enumerator objects)Select+ToArray(2 objects)For a typical search returning 10 results with 20 fields each:
HashSetallocations per page of results whenAllValues/GetValuesis accessed.GroupedAnd/Or/Notcall where the caller passesstring[].Trade-offs
fields as string[] ?? fields.ToArray()adds a type-check instruction (negligible overhead; faster than the allocation it skips).fields.Countmay over-allocate capacity by a few slots when field names repeat — this is a net positive since it avoids resizes.Reproducibility
Test Status
✅ Build clean (0 errors, 0 new warnings). All 147 tests passed (2 skipped as expected).
Add this agentic workflows to your repo
To install this agentic workflow, run