-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[browser][ST] trim ConcurrentDictionary #123330
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
base: main
Are you sure you want to change the base?
Conversation
43bc33f to
e4919ed
Compare
|
Tagging subscribers to this area: @dotnet/area-system-globalization |
|
Do you have any numbers for how much this saves for user apps? ConcurrentDictionary is used pretty frequently for performance reasons. It is not practical to ifdef out every use of it. If we are not happy about its footprint for single threaded wasm, it may be better to provide an alternative lighter weight implementation instead of trying to ifdef out every use. cc @stephentoub |
I would like to see number for a Blazor app. (total size / bytes saved by this change)
Ifdefs like this tend to keep growing to the point of being non-sustainable.
That's fine with me. Queue is used a lot less frequently and ThreadPool has a bunch of ifdefs for single-threaded wasm, so one more does not sound like a problem. |
|
I agree if there's a need for something like this, we should instead just have alternate impls of those data structures for the single threaded wasm use. Could be as simple as the public API shimming to a wrapped dictionary/queue. |

For single-threaded Browser runtime
EncodingTableandTimeZoneInfoConcurrentDictionaryand other items fromSystem.Collections.Concurrentassembly in the CoreLibConcurrentQueueinThreadPoolWorkQueueSystem.Runtime.Tests/TrimmingTests/DoesntIncludeConcurentTypes.csSystem.Collections.ConcurrentSystem.ComponentModel.TypeDescriptoronConcurrentDictionaryinILLink.Descriptors.LibraryBuild.xmlTypeDescriptoris already heavily marked withRequiresUnreferencedCodeAttribute