Skip to content
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

Deprecated 2D array extensions from Microsoft.Toolkit #3444

Merged

Conversation

Sergio0694
Copy link
Member

Part of #3062, related to #3435

PR Type

What kind of change does this PR introduce?

  • Optimization
  • Deprecation

What is the current behavior?

The Microsoft.Toolkit package has a number of extensions for 2D arrays that are inefficient (they're both a bit slow and causing unnecessary memory allocations) and replaced by equivalent APIs in the Microsoft.Toolkit.HighPerformance package.

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/30452cf0bbf627f12825cf50bbd31b0e526b9abe/Microsoft.Toolkit/Extensions/ArrayExtensions.cs#L27

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/30452cf0bbf627f12825cf50bbd31b0e526b9abe/Microsoft.Toolkit/Extensions/ArrayExtensions.cs#L48

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/30452cf0bbf627f12825cf50bbd31b0e526b9abe/Microsoft.Toolkit/Extensions/ArrayExtensions.cs#L68

What is the new behavior?

This PR makes these APIs obsolete, and also includes a number of small optimizations to existing code that was added during the refactoring to remove dependencies on these APIs, so that it'll be possible to remove them in the future with no issues.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

@Sergio0694 Sergio0694 added extensions ⚡ optimization ☄ Performance or memory usage improvements .NET Components which are .NET based (non UWP specific) labels Aug 20, 2020
@Sergio0694 Sergio0694 added this to the 7.0 milestone Aug 20, 2020
@ghost
Copy link

ghost commented Aug 20, 2020

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 ghost requested a review from Kyaa-dost August 20, 2020 22:24
@Sergio0694 Sergio0694 mentioned this pull request Aug 20, 2020
70 tasks
@Kyaa-dost Kyaa-dost added this to In progress in Technical 7.0 via automation Aug 25, 2020
@azchohfi
Copy link
Contributor

Tests are not passing.

@Sergio0694
Copy link
Member Author

*tests passing just fine*

"Hey looks like this code that is working perfectly fine now could potentially break, let's make this more reliable"

*proceeds to literally break the entire build*

@Sergio0694
Copy link
Member Author

Alright, all is well again 😄

@michael-hawker michael-hawker added reviewers wanted ✋ for-review 📖 To evaluate and validate the Issues or PR labels Sep 1, 2020
Copy link
Contributor

@Rosuavio Rosuavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the optimizations and the changes in the API. Good work!

This was referenced Mar 12, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merge ⚡ extensions ⚡ for-review 📖 To evaluate and validate the Issues or PR introduce breaking changes 💥 needs attention 👋 .NET Components which are .NET based (non UWP specific) optimization ☄ Performance or memory usage improvements
Projects
No open projects
Technical 7.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants