Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PERFORMANCE: Lucene.Net.Replicator.LocalReplicator::CheckExpiredSessions(): Remove unnecessary dictionary allocation #1070

Closed
1 task done
NightOwl888 opened this issue Dec 19, 2024 · 0 comments · Fixed by #1108
Assignees
Labels

Comments

@NightOwl888
Copy link
Contributor

NightOwl888 commented Dec 19, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Task description

In Lucene.Net.Replicator.LocalReplicator::CheckExpiredSessions(), there is an extra .ToArray() call that allocates a new collection to iterate the Values:

private readonly IDictionary<string, ReplicationSession> sessions = new Dictionary<string, ReplicationSession>();
/// <exception cref="InvalidOperationException"></exception>
private void CheckExpiredSessions()
{
// .NET NOTE: .ToArray() so we don't modify a collection we are enumerating...
// I am wondering if it would be overall more practical to switch to a concurrent dictionary...
foreach (ReplicationSession token in sessions.Values.Where(token => token.IsExpired(ExpirationThreshold)).ToArray())
{
ReleaseSession(token.Session.Id);
}
}

J2N 2.1 includes the update to J2N.Collections.Generic.Dictionary<TKey, TValue> that allows deleting while iterating forward through the collection in older target frameworks. So, swapping the implementation of the sessions field will allow us to remove this extra allocation (the call to ToArray()).

@NightOwl888 NightOwl888 added performance up-for-grabs This issue is open to be worked on by anyone good-first-issue Good for newcomers pri:normal is:task A chore to be done labels Dec 19, 2024
@NightOwl888 NightOwl888 added this to the 4.8.0-beta00018 milestone Dec 19, 2024
@NightOwl888 NightOwl888 self-assigned this Jan 20, 2025
@NightOwl888 NightOwl888 removed up-for-grabs This issue is open to be worked on by anyone good-first-issue Good for newcomers labels Jan 20, 2025
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Jan 20, 2025
…: Use J2N.Collections.Dictionary<TKey, TValue> to allow deleting while iterating forward prior to .NET Core (fixes apache#1070).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant