-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Remove use of Tuple<> from corelib #44706
Conversation
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 one area label. |
eb569f8
to
b4ceaeb
Compare
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 |
For uses that need to pass a few arguments through an object state, is there a measurable downside to boxing/unboxing a |
I doubt it that the difference is measurable. It is just an extra local copy (ie a few extra mov instructions). |
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. |
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
Show resolved
Hide resolved
b4ceaeb
to
3042cc6
Compare
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.)