-
Notifications
You must be signed in to change notification settings - Fork 789
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
Making ILVersionInfo a struct #6392
Conversation
Looks like I'll need to fix the tests. |
ready |
It would be good to get some data on what this saves. |
@cartermp for the heap dump we were reviewing, it is a 20MB reduction in reachable GC objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the change (even if perf was negligible, the readability improvements are great)
It would good to set up a quick benchmark to confirm we aren't getting a CPU time hit though
It's not really worth the benchmark IMO; 8 bytes falls under the 16 byte recommendation for structs so the performance impact if anything is practically none. |
@cartermp While I agree with you in general, keep in mind this is a targeted improvement. There are many places small reference types are used, but this is the one that is known to have the most direct impact on reachable set. The GC overhead improvement will easily offset any cost in creating or copying these objects. I'll work with @TIHan to do a better job of providing this information from the start in changes like this. |
@sharwell that you for your help on this. When we do go over it, my intention is to gain knowledge from you to be able to do a lot of this on my own. |
Also is it possible to target this to master and just let the bots move it over to 16.1? |
* Making ILVersionInfo a struct * Fixing tests
Making ILVersionInfo a struct (#6392)
This makes ILVersionInfo a strong struct type instead of a 4 item tuple. This should free up some memory by not making this an object.