-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Microsoft.Toolkit.HighPerformance package (part 2) #3273
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
Microsoft.Toolkit.HighPerformance package (part 2) #3273
Conversation
Changes from official master
Changes from master
Changes from master
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'm good with this if @azchohfi and/or @tannergooding are
@tannergooding this is the PR I mentioned the other day 😄 Other things you might be interested in are the |
Taking a look |
Skimmed most of it with a couple comments and reviewed the SIMD code specifically. Overall the logic looks good to me. |
Hey @tannergooding - thank you so much for taking the time to go through the PR, I really appreciate it! Glad to hear the overall logic seemed ok, I'll go through you various comments now 😄 |
@tannergooding thanks for taking a look for us! @Sergio0694 let me know when you've resolved all the discussions and we're good to merge this. |
@michael-hawker All good, I've added those extra remarks to Also made those refactorings that were mentioned (eg. integrating an As far as I can tell, the PR is ready to go now! 😄 |
@Sergio0694 let's get this in for now since things looked good to @tannergooding. And if we see some good usage of this new library we can investigate in the future spending more time for the complex optimization. Sound good? |
@michael-hawker Sounds perfect, absolutely! Also, mission complete! 🎉🎉🎉 This has been quite the journey, thank you, @azchohfi and @tannergooding so much! 😊 |
Follow up to #3128
PR Type
What kind of change does this PR introduce?
What is the new behavior?
This PR includes the following changes to the
Microsoft.Toolkit.HighPerformance
package:Below is a breakthrough of all the new types and APIs added in this PR:
Microsoft.Toolkit.HighPerformance.Buffers
Microsoft.Toolkit.HighPerformance.Extensions
Microsoft.Toolkit.HighPerformance.Helpers
PR Checklist
Please check if your PR fulfills the following requirements:
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 templatesIf new feature, add to Feature ListOther information
Marking as draft PR for now, so far I've added the follow up APIs and did all the various refactorings and bug fixes, but I still need to go through @michael-hawker's requested changes in #3128. Will remove the draft PR status when that part is done as well 🚀Done, ready for review! 🎉