Clean up NET6 and optimize NETSTANDARD#1072
Merged
Merged
Conversation
- Remove .NET 5/6/7 from `#if` directives and build - Delete commented-out file - Optimize NETSTANDARD encoding polyfills used in hot paths to eliminate intermediate array allocations - Fix `PeriodicTimer` CancellationTokenRegistration leak - Add encoding polyfill benchmark
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes framework conditionals around the new baseline (net8+) and improves NETSTANDARD hot-path performance by reducing intermediate allocations, while also cleaning up legacy/dead code.
Changes:
- Simplify/modernize
#ifdirectives by removing NET5/6/7-specific branches. - Optimize NETSTANDARD encoding polyfills to avoid intermediate
byte[]allocations in common single-segment paths andIBufferWriter<byte>writes. - Address
PeriodicTimercancellation registration lifetime handling and add a microbenchmark for the encoding polyfill changes.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/NATS.Client.Core/NatsMemoryOwner.cs | Updates runtime-comment to reflect net8+ assumptions for fast-span creation path. |
| src/NATS.Client.Core/NatsConnection.RequestReply.cs | Simplifies framework guard for string.Create usage. |
| src/NATS.Client.Core/NATS.Client.Core.csproj | Adjusts package dependency conditions to align with “prior to net8.0” baseline. |
| src/NATS.Client.Core/Internal/netstandard.cs | NETSTANDARD polyfill updates: encoding allocation reductions and PeriodicTimer registration tracking. |
| src/NATS.Client.Core/Internal/Telemetry.cs | Removes now-unneeded conditional compilation around ActivityContext.TryParse. |
| src/NATS.Client.Core/Internal/NatsPipeliningWriteProtocolProcessor.cs | Deletes fully commented-out legacy file. |
| src/Directory.Build.props | Moves trimming/AOT compatibility conditions to net8.0 baseline. |
| sandbox/MicroBenchmark/EncodingPolyfillBench.cs | Adds benchmarks comparing old vs new encoding polyfill approaches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Ensure static lambda used to avoid allocation.
mtmk
added a commit
that referenced
this pull request
May 27, 2026
* Add .NET 10 Target * Release 3.0.0-preview.1 (#1071) - Initial release with .NET 10 target - Also dropped .NET 6 target * Clean up NET6 and optimize NETSTANDARD (#1072) * Clean up NET6 and optimize NETSTANDARD - Remove .NET 5/6/7 from `#if` directives and build - Delete commented-out file - Optimize NETSTANDARD encoding polyfills used in hot paths to eliminate intermediate array allocations - Fix `PeriodicTimer` CancellationTokenRegistration leak - Add encoding polyfill benchmark * Fix `PeriodicTimer` cancellation to prevent leak - Ensure static lambda used to avoid allocation. * Update DI package dependencies and documentation (#1075) Switch project dependency from `NATS.Net` to `NATS.Client.Simplified` for better modularity. Add `NATS.Extensions.Microsoft.DependencyInjection` to `NATS.Net` dependencies. Update documentation to include comparisons between `NATS.Extensions.Microsoft.DependencyInjection` and `NATS.Client.Hosting`, clarifying usage scenarios. Add new DI guide to advanced docs. * Release 3.0.0-preview.2 (#1091) Merged main * Add message context to serialization interfaces (#1082) * Add support for optional NATS headers in serialization/deserialization * Make `Serialize` method header-aware and add unit tests for header serialization logic * Add recursion guard to prevent infinite loops in default interface methods for `Serialize`/`Deserialize`, update tests and examples accordingly * Fix `Content-Type` header deserialization by updating to use `StringValues` * Fix NuidTests flap * Improve NuidTests stability * Remove old default methods * Remove `IDictionary` implementation from `NatsHeaders` * Add INatsSerializeWithHeaders * serializer: remove unnecessary obsolete warning pragmas The CS0618 suppressions were leftover from the DIM approach where interface members were marked obsolete. No longer needed with the separate interface design. * tests: add serialization extension and ABI checks Add unit tests for NatsSerializationExtensions covering both the header-aware path and the fallback path for serialize and deserialize. Make the ABI check program assert correctness instead of just printing, including the new WithHeaders interfaces. * serializer: replace WithHeaders with WithContext interfaces Replace INatsSerializeWithHeaders<T>/INatsDeserializeWithHeaders<T> with INatsSerializeWithContext<T>/INatsDeserializeWithContext<T>, passing a NatsMsgContext struct that carries Subject, ReplyTo, and Headers. This gives serializers access to the full message envelope, enabling subject-based dispatch and reply-to inspection without requiring further interface changes in the future. Also fixes the null-headers asymmetry in the extension methods: both Serialize and Deserialize now always dispatch to the context overload when the interface is implemented, regardless of whether Headers is null. * Revert "Improve NuidTests stability" This reverts commit 42b8cbd. * Revert "Fix NuidTests flap" This reverts commit dcc3588. * docs: fix stale DIM references and update serialization docs Remove incorrect DIM claims from serialization docs, rewrite the headers section to describe the WithContext interface design. Clean up NativeAot example that had leftover INatsHeaders parameter overloads from the old DIM approach. Restore dropped <returns> doc comment on INatsDeserialize<T>.Deserialize. * headers: drop read-only state The read-only flag was set on NatsHeaders before passing to publish to catch accidental mutation after publish. It was a weak guarantee since headers are copied to a byte buffer synchronously inside CommandWriter, so post-publish mutation cannot affect wire data anyway. Removing it lets serializers implementing INatsSerializeWithContext<T> mutate headers (e.g. set Content-Type) during serialization, which was otherwise blocked by ThrowIfReadOnly. Telemetry's SetOverrideReadOnly escape hatch is no longer needed and uses the normal indexer instead. * tests: add net10.0 TFM and improve ABI checks Add net10.0 target to CoreUnit, Core, and Core2 test projects. Add CheckAbiOld control project that runs against the old NuGet version to prove type forwarding works by contrasting assembly versions. Centralize the old NuGet version in Directory.Build.props and make the run script assert loaded assembly versions. * tests: fix NU1510 package pruning errors on net10.0 Condition System.Net.Http and System.Text.RegularExpressions package references to net481 only, where they are actually needed. On net10.0 these are built-in and NuGet's package pruning treats the unnecessary references as errors. * serializers: address context review feedback - NatsMsgContext: require Subject via constructor; drop init setters so netstandard shim for IsExternalInit is no longer needed. - Add INatsSerializerWithContext<T> as a combined interface mirroring INatsSerializer<T>, simplifying class declarations for context-aware serializer+deserializer pairs. - Built-in NatsUtf8PrimitivesSerializer, NatsRawSerializer and NatsJsonContextSerializer now implement INatsSerializerWithContext<T> and propagate the context through _next when delegating. Without this, chaining a built-in with a context-aware leaf silently dropped context at the leaf. - Document NatsHeaders thread-safety: not safe to share across concurrent publishes, since readonly-locking was removed. - Document that header-mutating serializers must get non-null headers from the caller; the library does not allocate on their behalf. - Add chain-propagation tests for serialize and deserialize paths. * sln: remove x64/x86 platform noise * Release 3.0.0-preview.3 (#1113) * Use System.Threading.Lock on NET9_0_OR_GREATER (#1118) * core: use System.Threading.Lock on NET9_0_OR_GREATER * core: fix reentrant lock in PublishToClientHandlersAsync * bench: add lock and publish runtime comparison benchmarks * Release 3.0.0-preview.4 (#1123) * Release 3.0.0-preview.5 (#1130) * Release 3.0.0-preview.6 (#1141) * di: clarify package descriptions for NuGet [no ci]
This was referenced Jul 12, 2026
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#ifdirectives and buildPeriodicTimerCancellationTokenRegistration leakBenchmark results (GetBytes: ProtocolWriter/HeaderWriter hot path)
GetBytes (ProtocolWriter/HeaderWriter hot path — every pub/sub):
Using pointers eliminates the intermediate
byte[]allocation, with zero allocations. ~2.5x speedup across all sizes. This is the biggest win since these are called ~8 times per message in the protocol write path.GetString (HeaderParser/ReadProtocolProcessor — every message read):
Eliminates the
.ToArray()copy by using pointer overloads. The string itself still allocates (unavoidable), but removing the intermediate array saves ~45% of allocations and gives a ~1.5x speedup.GetBytesWriter (serialization path):
Writing directly to the
BufferWriterspan instead of allocating reduces more GC pressure and is 1.2x faster.All improvements only affect
NETSTANDARDbuilds (netstandard2.0/2.1) which is important in case we have any .net6 targeted applications which they would fallback to using netstandard2. Of course a bonus for .NET Framework applications too. The net8.0/net10.0 targets already use the built-in optimized APIs.