HighPerformance package improvements#3351
Conversation
|
Thanks Sergio0694 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
ghost
assigned
Sergio0694
added
feature 💡
high-performance 🚂
michael-hawker
left a comment
michael-hawker
left a comment
There was a problem hiding this comment.
@azchohfi this looks straight-forward enough to me.
Rosuavio
left a comment
Rosuavio
left a comment
There was a problem hiding this comment.
Really cool @Sergio0694 👍. I love work to minimize on unnecessary allocations. I am curious about switching from Unsafe.Unbox<T>(box) from what I read, it seemed pretty efficient.
| public static implicit operator T(Box<T> box) | ||
| { | ||
| return Unsafe.Unbox<T>(box); | ||
| return (T)(object)box; |
There was a problem hiding this comment.
I am curious what the difference is here.
There was a problem hiding this comment.
In theory the codegen should be identical, but this version has a better IL encoding which should help with scenarios such as R2R when using crossgen (which has some inlining limitations with method calls), as well as cases where the indirect load might cause stack spills or just not be optimized best by the JIT (especially on older runtimes).
The difference is that the previous version basically invoked that generic Unsafe.Unbox<T> and then issued a ldobj !!T instruction to dereference that T& value on the stack, whereas the second is simply resolved into a single unbox.any !!T opcode, which is the same being used by normal unbox operations. This change is mostly just a precaution to help crossgen and the JIT to always produce the most optimal codegen possible, it has no actual functional changes.
If you're interested, I made a small repro you can check out (here) - you can see that the asm x64 codegen is exactly the same on .NET Core 3.1, but if you switch to the IL tab you'll be able to see that different encoding I mentioned 😊
There was a problem hiding this comment.
I see, that's pretty cool (the change and sharplab.io). Thanks @Sergio0694 😃
|
Hello @Sergio0694! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 4 participants
Improvements for
Microsoft.Toolkit.HighPerformancePR Type
What kind of change does this PR introduce?
Changes and fixes
ArrayPool<T>overloads toMemoryOwner<T>PR Checklist
Please check if your PR fulfills the following requirements:
Pull Request has been submitted to the documentation repository instructions. Link:Sample in sample app has been added / updated (for bug fixes / features)Icon has been created (if new sample) following the Thumbnail Style Guide and templates