- Notifications
You must be signed in to change notification settings - Fork5.2k
Remove use of Tuple<> from corelib#44706
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Dotnet-GitSync-Bot commentedNov 15, 2020
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly onearea label. |
Uh oh!
There was an error while loading.Please reload this page.
jkotas commentedNov 15, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think this is a good idea. If we take this route, we should also do a pass to remove uses of Tuple in netcoreapp that are easy to avoid. For example, the Tuple use in |
stephentoub commentedNov 15, 2020
For uses that need to pass a few arguments through an object state, is there a measurable downside to boxing/unboxing a |
jkotas commentedNov 15, 2020
I doubt it that the difference is measurable. It is just an extra local copy (ie a few extra mov instructions). |
jkotas commentedNov 15, 2020
There is a difference between TupleSlim vs. Tuple/ValueTuple for AOT. The full AOT compilers will typically end up generating the code for all interfaces and methods on Tuple/ValueTuple. TupleSlim avoids this overhead. |
stephentoub commentedNov 15, 2020
Right.
Minus anything that could be trimmed, but presumably the problem here is little can be trimmed because most of the methods are interface implementations or virtual overrides.
And if it were an issue (which it shouldn't be), there's always Unsafe.Unbox I guess. |
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Related to#44684
cc:@jkotas,@marek-safar
(The flip side of this change, of course, is that if
Tuple<T1, T2>is already being used by the rest of the app, this could actually increase the overall size rather than decrease it.)