Allow user properties as pre-encoded UTF-8 binary buffer #2228
Conversation
|
@dotnet-policy-service agree |
|
There is a breaking change in dotnet 10, that doesn't allow VSTest runner when using dotnet test. It is recommended to switch to Microsoft.Test.Platform (MTP). I made the required changes in this PR, that the test github workflow can run successfully. Details under: https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-with-dotnet-test |
|
Avoid failing build workflow, by only signing the package when executing the pipeline from the main repo, not from a fork |
| public string Value { get; } | ||
| public ReadOnlyMemory<byte> ValueBuffer => _valueBuffer; | ||
|
|
||
| public string Value => this.ReadValueAsString(); |
There was a problem hiding this comment.
Please mark this as obsolete so that users will migrate to the new extension method.
| readonly ReadOnlyMemory<byte> _valueBuffer; | ||
|
|
||
| public MqttUserProperty(string name, string value) | ||
| : this(name, new ReadOnlyMemory<byte>(Encoding.UTF8.GetBytes(value ?? throw new ArgumentNullException(nameof(value))))) |
There was a problem hiding this comment.
Please mark this constructor as obsolete so that users will use the builders like MqttApplicationMessageBuilder which supports strings etc.
2 participants
Issue:
When extensively using MQTT v5 user properties, handling the key and value as string has two major downsides (allocation time and garbage collection). Keys can use string interning or defined as const but the values (when they are changing) can cause high allocation rate and latency spikes when the garbage collector has to collect them.
Proposal:
Allow value of MQTT v5 user properties to be added as bytes (pre-encoded UTF-8 binary buffer), using performance optimized dotnet types like ReadOnlyMemory. The same optimization was already implemented in version 5.x for the payload of the MQTT message itself.
Outlook:
The current implementation optimizes the MQTT message send path, it would be also possible to optimize the receive path and handling values of user properties only as ReadOnlyMemory and just allocate the string on demand. I didn't implement that yet, as this would require more API changes and might even have negative consequences (when user property values are read multiple times, caching would be required).
Pull Request contains the implementation of the proposal, including tests for binary user properties, a sample demonstrating publishing a message with binary user properties (
Samples/Client/Client_Publish_Samples.cs) and minor fixes for consistency and correctness.