apache / apache/lucenenet

Use concrete collection types for private fields and variables

Open
#1,201 7 comments 1 reaction 0 assignees View on GitHub
good-first-issue is:task performance pri:low up-for-grabs
Dominant language
C#
Stars
2.4k
Forks
658
Avg merge
3d 5h
Merged PRs (30d)
9

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Task description

I noticed when reviewing #1200 that the [existing code (example)](https://github.com/apache/lucenenet/pull/1200/files#diff-092b352b0e745a7f63744637e0e854c22e44fe2f12624b8f95640ac0327de86cR80) sometimes uses interfaces like `IDictionary` for the type of private fields and variables, instead of the concrete type. While this matches upstream and helps ensure you're coding to the contract rather than the implementation, this might be accidentally harming performance.

By typing the field/variable to the interface, we lose out on some optimizations like how [`OrderedDictionary` uses a struct enumerator to avoid an allocation/boxing](https://github.com/NightOwl888/J2N/blob/main/src/J2N/Collections/Generic/OrderedDictionary.cs#L1373). Also, the JIT can more aggressively inline, eliminate bounds checks, vectorize, etc. when the type is concrete.

Microsoft has the [CA1859](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1859) analyzer specifically to flag this, and this can be used to make finding issues and refactoring easier.

Note that we should only do this for **private** fields (and potentially other private members, like return types of private methods) as well as variables, so that we are not changing the public contracts. A comment should be added mentioning CA1859, i.e. `// LUCENENET: CA1859 - Use concrete types when possible for improved performance`

While there is likely benefit beyond collections, let's start small and not "boil the ocean" here. Collections are where there is the biggest benefit due to things like stack allocated enumerators and possible vectorization.

I think this is worth evaluating as a deviation from exactly matching upstream Lucene code. cc @NightOwl888 for thoughts/feedback.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the collection declaration in the code example from PR #1200 and the CA1859 guidance. Inventory private collection fields, private members, and local variables without changing public contracts; done means concrete types are used where appropriate and each deviation is marked with the requested CA1859 comment.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.