## PR type:
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->
- Performance improvement
<!-- - Bugfix -->
<!-- - Feature -->
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying, or link to a relevant issue. -->
The `MemoryStream` type wraps a `Memory<byte>` instance, so every time a read/write operation needs to be performed, it will need to use `Memory<T>.Span`, which is relatively expensive to invoke, as it needs to handle multiple possible sources of data. We can use generic types and the value delegate trick (constrained generic struct + interface) to optimize this away.
## What is the new behavior?
<!-- Describe how was this issue resolved or changed? -->
We now use a `MemoryStream<TSource>` type (which is internal anyway) and have a bunch of `IMemorySource` types specifically supporting `byte[]` and `MemoryManager<byte>` sources with additional optimizations wherever possible. Also, just using this already removes all the overhead of dynamically creating a `Span<T>` from the wrapped `Memory<T>` instance.
Here's a benchmark I put together to show the before/after:
The benchmark consists of writing 1024 times a 1024 bytes buffer to a target `Stream` instance.
The new `MemoryStream<TSource>` version is almost 25% faster just thanks to this change 🚀
## PR Checklist
Please check if your PR fulfills the following requirements:
- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs).~~
- [ ] ~~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](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [X] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes
## Overview
Some `Guard` APIs were incorrectly using the wrong capitalization for the `WhiteSpace` part, not following the same structure of the actual APIs in the BCL (eg. `string.IsNullOrWhiteSpace`). This PR deprecates them and adds new APIs with the right name.
...I'm sorry 😅
<!-- Add a brief overview here of the feature/bug & fix. -->
## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->
- API rename
- API deprecation
## PR Checklist
Please check if your PR fulfills the following requirements:
- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR 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](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [ ] ~~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*)~~
- [X] Contains **NO** breaking changes
## Follow up for #3131
<!-- Add a brief overview here of the feature/bug & fix. -->
The `Type.ToTypeString()` introduced with the `Guard` API lacks support for nested types, pointer types and byref types.
## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->
<!-- - Bugfix -->
<!-- - Feature -->
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying, or link to a relevant issue. -->
The extension doesn't work properly when used with nested, pointer or byref types.
It will either crash or just return the wrong formatted type (eg. the declaring type instead of the nested type).
## What is the new behavior?
<!-- Describe how was this issue resolved or changed? -->
The extension now works correctly with all sorts of nested, pointer and byref types 🎉
## PR Checklist
Please check if your PR fulfills the following requirements:
- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs).~~
- [ ] ~~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](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [X] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes
## Follow up for #3230, part of #3428
<!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes#1234) which will automatically close the issue once the PR is merged. -->
<!-- Add a brief overview here of the feature/bug & fix. -->
## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->
- Optimization
<!-- - Bugfix -->
<!-- - Feature -->
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->
## Overview
This PR includes two main parts: an improvement for the memory pooling buffers, and a refactor of how delegates are registered for message handlers. This last part allows to completely remove local closures for message handlers 🚀
### Memory pooling improvements
We're using memory pooling (by relying on the `ArrayPool<T>` APIs) in the `Messenger` class to achieve an amortized 0-allocation execution of the `Send` method in particular (`UnregisterAll` uses this too). We have code like this:
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/4890b5ed17995654b6f710b7949d1786648317a4/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs#L250https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/4890b5ed17995654b6f710b7949d1786648317a4/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs#L401
This works just fine, and we do get the 0-allocation after the initial first invocation where we rent the buffer.
There are two downsides here though:
- In the `Send` method, we rent an `Action<TMessage>` buffer, so we'll rent a different buffer for every message type
- Given that these types are either internal or not likely to ever by used by consumers of the library, all these rented buffers would only ever by used by the `Messenger` class itself, and the consumers would not be able to reuse them on their own.
Neither of these points is a big deal and the current implementation is fine, but I think we can do better 😄
**Idea:** let's leverage the fact that arrays are covariant, and only use a single type to solve both problems, like so:
```csharp
object[] maps = ArrayPool<object>.Shared.Rent(count);
// Do stuff, example:
foreach (object obj in maps)
{
var myActualItem = (SomeFancyTypePossiblyInternal)obj;
// Do stuff with the item...
}
```
This both allows us to just use `object[]` arrays, which both reduces the total number of rented arrays (as we can reuse them for different message types), and also makes it so that these rented arrays might potentially also be reused by consumers of the library (should they ever need to pool `object[]` arrays), further reducing allocations 🚀
## Benchmarks
Here are some benchmarks comparing the `Messenger` from this PR with the one in the `Preview1`.
### Sending messages
| Method | Categories | Mean | Error | StdDev | Ratio | Gen 0 | Gen 1 | Allocated |
|--------- |----------------- |-------------:|----------:|----------:|------:|------:|------:|----------:|
| Old_Send | DefaultChannel | 161.7 us | 1.52 us | 1.43 us | 1.00 | - | - | - |
| **New_Send** | DefaultChannel | **153.9 us** | 1.62 us | 1.43 us | 0.95 | - | - | - |
| | | | | | | | | |
| Old_Send | MultipleChannels | 148,668.7 us | 886.65 us | 785.99 us | 1.00 | - | - | 336 B |
| **New_Send** | MultipleChannels | **138,825.0 us** | 797.61 us | 746.08 us | 0.93 | - | - | 336 B |
Performance when sending message is slightly faster than before. Worst case scenario, it's not any slower.
### Registering messages
| Method | Mean | Error | StdDev | Ratio | Gen 0 | Gen 1 | Allocated |
|------------- |---------:|--------:|--------:|------:|---------:|--------:|----------:|
| Old_Register | 443.6 us | 1.73 us | 1.53 us | 1.00 | 113.7695 | 25.3906 | 581.53 KB |
| **New_Register** | **386.1 us** | 2.47 us | 2.31 us | 0.87 | 82.5195 | 4.3945 | **381.53 KB** |
The new version is **13%** faster when registering messages, and uses **34%** less memory 🚀
### Enabled recipient type-specific handlers
One major annoyance for users working with manually registered handlers was the fact that type information was lost.
As in, recipients were registered as just `object` instances, as it was necessary to cast them in the handler every time.
This PR also changes this by adding support for a type parameter to specify the recipient type.
This enables the following change (which is totally optional anyway, you can still just use `object` if you want):
```csharp
// Before
Messenger.Register<MyMessage>(this, (s, m) => ((MyViewModel)s).SomeMethod(m));
// After
Messenger.Register<MyViewModel, MyMessage>(this, (s, m) => s.SomeMethod(m));
```
### Removed local closures
The original implementation used `Action<T>` for handlers, which caused closures to be constantly created whenever the users wanted to access any local member on the registered recipient. This was because the recipient itself needed to be captured too to be accessed from the handlers. This detail also made it more difficult for other devs to implement `IMessenger` if they wanted to use a weak reference system. I've replaced that handler type with a custom delegate, called `MessageHandler`:
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/204ee41ad285bb960876a303ce83d1a5bc454e1a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs#L10-L22
This delegate also receives the target recipient as input, which allows developers to just use that to access local members, without the need to create closures. The handlers are now essentially static, so the C# compiler can cache the whole delegate too. This is especially useful for the `IRecipient<TMessage>` pattern, as that was previously creating unnecessary closures when registering handlers - that's completely gone now and delegates are cached there as well 🎉
For instance, you can see that here:
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/204ee41ad285bb960876a303ce83d1a5bc454e1a/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs#L175-L179
We're now using that cached static delegate (will be able to also explicitly mark that as static when C# 9 lands, but it already is) instead of the method group syntax on `recipient.Receive`, which allocated a closure for each invocation.
## PR Checklist
Please check if your PR fulfills the following requirements:
- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR 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](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [X] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes
## Improvements for `Microsoft.Toolkit.HighPerformance`
<!-- Add a brief overview here of the feature/bug & fix. -->
## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->
<!-- - Bugfix -->
- Feature
- Optimization
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->
## Changes and fixes
<!-- Describe how was this issue resolved or changed? -->
- Improved codegen for exception throw methods
- Added custom `ArrayPool<T>` overloads to `MemoryOwner<T>`
## PR Checklist
Please check if your PR fulfills the following requirements:
- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR 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](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [ ] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes
<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below.
Please note that breaking changes are likely to be rejected. -->
## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->
<!-- - Bugfix -->
- Feature
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->
## What is the new behavior?
<!-- Describe how was this issue resolved or changed? -->
This PR adds a few new APIs to the `BitHelper` class.
Follow up from this original thread on Twitter: https://twitter.com/SergioPedri/status/1275556765363568641.
These are the new APIs being included in this PR:
```csharp
namespace Microsoft.Toolkit.HighPerformance.Helpers
{
public static class BitHelper
{
public static bool HasZeroByte(uint value);
public static bool HasZeroByte(ulong value);
public static bool HasByteEqualTo(uint value, byte target);
public static bool HasByteEqualTo(ulong value, byte target);
}
}
```
## PR Checklist
Please check if your PR fulfills the following requirements:
- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR 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](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [X] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes
<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below.
Please note that breaking changes are likely to be rejected. -->
Sergio Pedri
