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

.net Core and performance increase #15

Open
JCKodel opened this issue Jul 6, 2017 · 5 comments
Open

.net Core and performance increase #15

JCKodel opened this issue Jul 6, 2017 · 5 comments

Comments

@JCKodel
Copy link
Contributor

JCKodel commented Jul 6, 2017

The code is .net core 1.1 compatible, please, update the nuget =)

And why using ConcurrentDictionary<K,V> on a single thread environment? It is WAY slower then the default Dictionary<K, V>: https://stackoverflow.com/questions/15252115/concurrentdictionary-performance-at-a-single-thread-misunderstanding

@christianrondeau
Copy link
Collaborator

Good idea for .Net Core, especially if it's "free" :) If you already know how to enable .Net Core in NuGet packaging and in the build, please feel free to do a pull request for a quicker update ;)

As for the ConcurrentDictionary, it is not thread-safe (we cache the mapping once).

It might be cheaper to re-create the dictionary every time however, it would be worth doing some tests. Again, if you want to do a PR for this, I'd like to see some numbers :) But the dictionary cannot be static if we do that.

@JCKodel
Copy link
Contributor Author

JCKodel commented Jul 7, 2017

Unfortunately, I never played with nuget before, so I don't know how to update it. :(

For the dictionary, correct me if I'm wrong, but we're just reading it (except, of course, in the initialization), so, it should be thread safe:

https://msdn.microsoft.com/en-us/library/xfhwa508(v=vs.110).aspx

Thread Safety
A Dictionary<TKey, TValue> can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

@christianrondeau
Copy link
Collaborator

You may be right. The original code actually did create itself at runtime, and I believe we could indeed bring back a normal dictionary since it's initialized by the JIT (because it's statically initialized, it is guaranteed to be thread-safe).

So, both of your requests are totally valid :)

@christianrondeau
Copy link
Collaborator

I did some quick tests:

With ConcurrentDictionary:
Did 1024 compressions in 16.6660174s. Average: 16ms
Did 1024 decompressions in 0.3325204s. Average: 0ms

With normal Dictionary:
Did 1024 compressions in 16.872454s. Average: 16ms
Did 1024 decompressions in 0.2893404s. Average: 0ms

Seems like performances are pretty much the same. I'll probably push this anyway... later :) If you feel like looking into the .Net Core stuff, that'd be nice :)

@JCKodel
Copy link
Contributor Author

JCKodel commented Jul 7, 2017

#16

Converted to .net Standard 1.0 (https://docs.microsoft.com/en-us/dotnet/standard/net-standard).

Unit tests run successfully, but I can't help with the build script (pretty sure that will never work on Linux and MacOS).

As far I researched, nuget package doesn't need anything special (only a .net Portable Library (deprecated) or a .net Standard project (.net Core compliant)).

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

No branches or pull requests

2 participants