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

More idiomatic usage of ConcurrentDictionary #2221

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

jnyrup
Copy link
Contributor

@jnyrup jnyrup commented Jun 7, 2023

Found some places, where we can use built-in ConcurrentDictionary methods to reduce the number of lookups.
It might also improve thread-safety a tad, as currently a value could theoretically be removed/added between calls to ContainsKey and TryGetValue.

I refrained from updating ChromeTargetManager.OnTargetCreated and ChromeTargetManager.OnAttachedToTarget as the valueFactory might be evaluated more than once, to ensure that _targetFactoryFunc is still only evaluated once.

If you call GetOrAdd simultaneously on different threads, valueFactory may be called multiple times, but only one key/value pair will be added to the dictionary.

https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.getoradd?view=net-7.0#system-collections-concurrent-concurrentdictionary-2-getoradd(-0-system-func((-0-1)))

Copy link
Member

@kblok kblok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice findings!

@kblok kblok merged commit c78f094 into hardkoded:master Jun 8, 2023
@jnyrup jnyrup deleted the concurrent_dictionary branch June 8, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants