From 0ece8ea819d29875203b37b61ec6e83b05de42a5 Mon Sep 17 00:00:00 2001 From: jasper-d Date: Tue, 5 Sep 2023 23:12:00 +0200 Subject: [PATCH 1/7] Add NuidWriter --- sandbox/MicroBenchmark/MicroBenchmark.csproj | 2 + sandbox/MicroBenchmark/NewInboxBenchmarks.cs | 58 +++ sandbox/MicroBenchmark/Program.cs | 4 + src/NATS.Client.Core/Internal/NuidWriter.cs | 140 +++++++ .../Internal/SubscriptionManager.cs | 2 +- src/NATS.Client.Core/NATS.Client.Core.csproj | 4 +- .../NatsConnection.RequestReply.cs | 42 +- .../NatsConnection.RequestSub.cs | 2 +- src/NATS.Client.Core/NatsConnection.cs | 2 +- .../NATS.Client.Core.Tests/NuidWriterTests.cs | 373 ++++++++++++++++++ 10 files changed, 624 insertions(+), 5 deletions(-) create mode 100644 sandbox/MicroBenchmark/NewInboxBenchmarks.cs create mode 100644 src/NATS.Client.Core/Internal/NuidWriter.cs create mode 100644 tests/NATS.Client.Core.Tests/NuidWriterTests.cs diff --git a/sandbox/MicroBenchmark/MicroBenchmark.csproj b/sandbox/MicroBenchmark/MicroBenchmark.csproj index 53579c16d..19703b686 100644 --- a/sandbox/MicroBenchmark/MicroBenchmark.csproj +++ b/sandbox/MicroBenchmark/MicroBenchmark.csproj @@ -3,9 +3,11 @@ Exe net6.0 + net6.0;net8.0 enable enable false + true diff --git a/sandbox/MicroBenchmark/NewInboxBenchmarks.cs b/sandbox/MicroBenchmark/NewInboxBenchmarks.cs new file mode 100644 index 000000000..826ae0fdd --- /dev/null +++ b/sandbox/MicroBenchmark/NewInboxBenchmarks.cs @@ -0,0 +1,58 @@ +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; +using System.Security.Cryptography; +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Diagnosers; +using BenchmarkDotNet.Jobs; +using NATS.Client.Core; +using NATS.Client.Core.Internal; + +namespace MicroBenchmark; + +[MemoryDiagnoser] + +[SimpleJob(RuntimeMoniker.Net80)] +[SimpleJob(RuntimeMoniker.NativeAot80)] +[SimpleJob(RuntimeMoniker.Net60)] +[SimpleJob(RuntimeMoniker.Net70, baseline: true)] +public class NewInboxBenchmarks +{ + private char[] buf = new char[32]; + + private static readonly NatsOpts s_longPrefixOpt = NatsOpts.Default + with + { + InboxPrefix = "this-is-a-rather-long-prefix-that-we-use-here" + }; + + private static readonly NatsConnection _connectionDefaultPrefix = new(); + private static readonly NatsConnection _connectionLongPrefix = new(s_longPrefixOpt); + + [GlobalSetup] + public void Setup() + { + NuidWriter.TryWriteNuid(new char[100]); + } + + [Benchmark(Baseline = true)] + [SkipLocalsInit] + public bool TryWriteNuid() + { + return NuidWriter.TryWriteNuid(buf); + } + + [Benchmark] + public string NewInbox_ShortPrefix() + { + return _connectionDefaultPrefix.NewInbox(); + } + + [Benchmark] + public string NewInbox_LongPrefix() + { + return _connectionLongPrefix.NewInbox(); + } +} diff --git a/sandbox/MicroBenchmark/Program.cs b/sandbox/MicroBenchmark/Program.cs index c9a046727..a9806db04 100644 --- a/sandbox/MicroBenchmark/Program.cs +++ b/sandbox/MicroBenchmark/Program.cs @@ -1,3 +1,7 @@ using BenchmarkDotNet.Running; BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args); + + + + diff --git a/src/NATS.Client.Core/Internal/NuidWriter.cs b/src/NATS.Client.Core/Internal/NuidWriter.cs new file mode 100644 index 000000000..f802137a4 --- /dev/null +++ b/src/NATS.Client.Core/Internal/NuidWriter.cs @@ -0,0 +1,140 @@ +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; +using System.Security.Cryptography; + +namespace NATS.Client.Core.Internal; + +[SkipLocalsInit] +internal sealed class NuidWriter +{ + private const nuint BASE = 62; + private const ulong MAX_SEQUENTIAL = 839299365868340224; // 62^10 // 0x1000_0000_0000_0000; // 64 ^10 + private const uint PREFIX_LENGTH = 12; + private const nuint SEQUENTIAL_LENGTH = 10; + private const int MIN_INCREMENT = 33; + private const int MAX_INCREMENT = 333; + internal const nuint NUID_LENGTH = PREFIX_LENGTH + SEQUENTIAL_LENGTH; + + [ThreadStatic] + private static NuidWriter? t_writer; + + // TODO: Use UTF8 string literal when upgrading to .NET 7+ + private static ReadOnlySpan Digits => "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + + private char[] _prefix; + private ulong _increment; + private ulong _sequential; + + internal static int PrefixLength => (int)PREFIX_LENGTH; + + private NuidWriter() + { + Refresh(out _); + } + + public static bool TryWriteNuid(Span nuidBuffer) + { + if(t_writer is not null) + { + return t_writer.TryWriteNuidCore(nuidBuffer); + } + + return InitAndWrite(nuidBuffer); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool InitAndWrite(Span span) + { + t_writer = new NuidWriter(); + return t_writer.TryWriteNuidCore(span); + } + + private bool TryWriteNuidCore(Span nuidBuffer) + { + ulong sequential = _sequential += _increment; + + if(sequential < MAX_SEQUENTIAL) + { + return TryWriteNuidCore(nuidBuffer, _prefix, sequential); + } + + return RefreshAndWrite(nuidBuffer); + + [MethodImpl(MethodImplOptions.NoInlining)] + bool RefreshAndWrite(Span buffer) + { + char[] prefix = Refresh(out sequential); + return TryWriteNuidCore(buffer, prefix, sequential); + } + } + + private static bool TryWriteNuidCore(Span buffer, Span prefix, ulong sequential) + { + if ((uint)buffer.Length < NUID_LENGTH || prefix.Length != PREFIX_LENGTH || (uint)prefix.Length > (uint)buffer.Length) + { + return false; + } + + Unsafe.CopyBlockUnaligned(ref Unsafe.As(ref buffer[0]), ref Unsafe.As(ref prefix[0]), PREFIX_LENGTH * sizeof(char)); + + // NOTE: We must never write to digitsPtr! + ref char digitsPtr = ref MemoryMarshal.GetReference(Digits); + + for(nuint i = PREFIX_LENGTH; i < NUID_LENGTH; i++) + { + nuint digitIndex = (nuint)(sequential % BASE); + Unsafe.Add(ref buffer[0], i) = Unsafe.Add(ref digitsPtr, digitIndex); + sequential /= BASE; + } + + return true; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + [MemberNotNull(nameof(_prefix))] + private char[] Refresh(out ulong sequential) + { + char[] prefix = _prefix = GetPrefix(); + _increment = GetIncrement(); + sequential = _sequential = GetSequential(); + return prefix; + } + + private static uint GetIncrement() + { + return (uint)Random.Shared.Next(MIN_INCREMENT, MAX_INCREMENT + 1); + } + + private static ulong GetSequential() + { + return (ulong)Random.Shared.NextInt64(0, (long)MAX_SEQUENTIAL + 1); + } + + private static char[] GetPrefix(RandomNumberGenerator? rng = null) + { + Span randomBytes = stackalloc byte[(int)PREFIX_LENGTH]; + + // TODO: For .NET 8+, use GetItems for better distribution + if(rng == null) + { + RandomNumberGenerator.Fill(randomBytes); + } + else + { + rng.GetBytes(randomBytes); + } + + char[] newPrefix = new char[PREFIX_LENGTH]; + + for(int i = 0; i < randomBytes.Length; i++) + { + int digitIndex = (int)(randomBytes[i] % BASE); + newPrefix[i] = Digits[digitIndex]; + } + + return newPrefix; + } +} + diff --git a/src/NATS.Client.Core/Internal/SubscriptionManager.cs b/src/NATS.Client.Core/Internal/SubscriptionManager.cs index 561e987eb..39bfcd52c 100644 --- a/src/NATS.Client.Core/Internal/SubscriptionManager.cs +++ b/src/NATS.Client.Core/Internal/SubscriptionManager.cs @@ -180,7 +180,7 @@ private async ValueTask SubscribeInboxAsync(string subject, NatsSubOpts? opts, N { if (Interlocked.CompareExchange(ref _inboxSub, _inboxSubSentinel, _inboxSubSentinel) == _inboxSubSentinel) { - var inboxSubject = $"{_inboxPrefix}*"; + var inboxSubject = $"{_inboxPrefix}.*"; _inboxSub = InboxSubBuilder.Build(subject, opts, _connection, manager: this); await SubscribeInternalAsync( inboxSubject, diff --git a/src/NATS.Client.Core/NATS.Client.Core.csproj b/src/NATS.Client.Core/NATS.Client.Core.csproj index 84d67fa0b..7784f29af 100644 --- a/src/NATS.Client.Core/NATS.Client.Core.csproj +++ b/src/NATS.Client.Core/NATS.Client.Core.csproj @@ -1,7 +1,7 @@  - net6.0 + net6.0;net7.0;net8.0 enable enable true @@ -20,6 +20,8 @@ runtime; build; native; contentfiles; analyzers; buildtransitive + + diff --git a/src/NATS.Client.Core/NatsConnection.RequestReply.cs b/src/NATS.Client.Core/NatsConnection.RequestReply.cs index c6fb41a1f..fdcefdc4f 100644 --- a/src/NATS.Client.Core/NatsConnection.RequestReply.cs +++ b/src/NATS.Client.Core/NatsConnection.RequestReply.cs @@ -1,4 +1,8 @@ +using System.Buffers; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; +using NATS.Client.Core.Internal; namespace NATS.Client.Core; @@ -7,7 +11,43 @@ public partial class NatsConnection private static readonly NatsSubOpts DefaultReplyOpts = new() { MaxMsgs = 1 }; /// - public string NewInbox() => $"{InboxPrefix}{Guid.NewGuid():n}"; + public string NewInbox() => NewInbox(InboxPrefix); + + [SkipLocalsInit] + private static string NewInbox(ReadOnlySpan prefix) + { + Span buffer = stackalloc char[64]; + uint separatorLength = prefix.Length > 0 ? 1u : 0u; + uint totalLength = (uint)prefix.Length + (uint)NuidWriter.NUID_LENGTH + separatorLength; + if (totalLength <= buffer.Length) + { + buffer = buffer.Slice(0, (int)totalLength); + } + else + { + buffer = new char[totalLength]; + } + + uint totalPrefixLength = (uint)prefix.Length + separatorLength; + if ((uint)buffer.Length > totalPrefixLength && (uint)buffer.Length > (uint)prefix.Length) + { + prefix.CopyTo(buffer); + buffer[prefix.Length] = '.'; + Span remaining = buffer.Slice((int)totalPrefixLength); + bool didWrite = NuidWriter.TryWriteNuid(remaining); + Debug.Assert(didWrite, "didWrite"); + return new string(buffer); + } + + return Throw(); + + [DoesNotReturn] + string Throw() + { + Debug.Fail("Must not happen"); + throw new InvalidOperationException("This should never be raised!"); + } + } /// public async ValueTask?> RequestAsync( diff --git a/src/NATS.Client.Core/NatsConnection.RequestSub.cs b/src/NATS.Client.Core/NatsConnection.RequestSub.cs index 50a84fa26..f9f17b0bd 100644 --- a/src/NATS.Client.Core/NatsConnection.RequestSub.cs +++ b/src/NATS.Client.Core/NatsConnection.RequestSub.cs @@ -10,7 +10,7 @@ internal async ValueTask> RequestSubAsync( NatsSubOpts? replyOpts = default, CancellationToken cancellationToken = default) { - var replyTo = $"{InboxPrefix}{Guid.NewGuid():n}"; + var replyTo = NewInbox(); var replySerializer = replyOpts?.Serializer ?? Opts.Serializer; var sub = new NatsSub(this, SubscriptionManager.InboxSubBuilder, replyTo, queueGroup: default, replyOpts, replySerializer); diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index 0d5d6c9d0..7e036dbae 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -68,7 +68,7 @@ public NatsConnection(NatsOpts opts) Counter = new ConnectionStatsCounter(); _writerState = new WriterState(opts); _commandWriter = _writerState.CommandBuffer.Writer; - InboxPrefix = $"{opts.InboxPrefix}.{Guid.NewGuid():n}."; + InboxPrefix = NewInbox(opts.InboxPrefix); SubscriptionManager = new SubscriptionManager(this, InboxPrefix); _logger = opts.LoggerFactory.CreateLogger(); _clientOpts = ClientOpts.Create(Opts); diff --git a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs new file mode 100644 index 000000000..cd8aa426b --- /dev/null +++ b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs @@ -0,0 +1,373 @@ +using System.Collections.Concurrent; +using System.Diagnostics; +using System.Reflection; +using System.Security.Cryptography; +using System.Text; +using System.Text.RegularExpressions; + +namespace NATS.Client.Core.Tests; + +public class NuidWriterTests +{ + private static readonly Regex _nuidRegex = new Regex("[A-z0-9]{22}"); + + private readonly ITestOutputHelper _outputHelper; + + public NuidWriterTests(ITestOutputHelper outputHelper) + { + _outputHelper = outputHelper; + } + + [Theory] + [InlineData(default(string))] + [InlineData("")] + [InlineData("__INBOX")] + [InlineData("long-inbox-prefix-above-stackalloc-limit-of-64")] + public void NewInbox_NuidAppended(string? prefix) + { + var natsOpts = NatsOpts.Default with { InboxPrefix = prefix }; + var sut = new NatsConnection(natsOpts); + + var inbox = sut.InboxPrefix; + var newInbox = sut.NewInbox(); + + Assert.Matches($"{prefix ?? ""}{(prefix?.Length > 0 ? "." : "")}[A-z0-9]{{22}}", inbox); + Assert.Matches($"{prefix ?? ""}{(prefix?.Length > 0 ? "." : "")}[A-z0-9]{{22}}.[A-z0-9]{{22}}", newInbox); + _outputHelper.WriteLine($"Prefix: '{prefix}'"); + _outputHelper.WriteLine($"Inbox: '{inbox}'"); + _outputHelper.WriteLine($"NewInbox: '{newInbox}'"); + } + + [Fact] + public void GetNextNuid_ReturnsNuidOfLength22_Char() + { + // Arrange + Span buffer = stackalloc char[44]; + + // Act + bool result = NuidWriter.TryWriteNuid(buffer); + + // Assert + ReadOnlySpan lower = buffer.Slice(0, 22); + string resultAsString = new(lower); + ReadOnlySpan upper = buffer.Slice(22); + + Assert.True(result); + + Assert.Matches("[A-z0-9]{22}", resultAsString); + Assert.All(upper.ToArray(), b => Assert.Equal(0, b)); + } + + [Fact] + public void GetNextNuid_BufferToShort_False_Char() + { + // Arrange + Span nuid = stackalloc char[(int)NuidWriter.NUID_LENGTH - 1]; + + // Act + bool result = NuidWriter.TryWriteNuid(nuid); + + // Assert + Assert.False(result); + Assert.All(nuid.ToArray(), b => Assert.Equal(0, b)); + } + + [Fact] + public void GetNextNuid_ReturnsDifferentNuidEachTime_Char() + { + // Arrange + Span firstNuid = stackalloc char[22]; + Span secondNuid = stackalloc char[22]; + + // Act + bool result = NuidWriter.TryWriteNuid(firstNuid); + result &= NuidWriter.TryWriteNuid(secondNuid); + + // Assert + Assert.False(firstNuid.SequenceEqual(secondNuid)); + Assert.True(result); + } + + [Fact] + public void GetNextNuid_PrefixIsConstant_Char() + { + // Arrange + Span firstNuid = stackalloc char[22]; + Span secondNuid = stackalloc char[22]; + + // Act + bool result = NuidWriter.TryWriteNuid(firstNuid); + result &= NuidWriter.TryWriteNuid(secondNuid); + + // Assert + Assert.True(result); + Assert.True(firstNuid.Slice(0, 12).SequenceEqual(secondNuid.Slice(0, 12))); + } + + [Fact] + public void GetNextNuid_ContainsOnlyValidCharacters_Char() + { + // Arrange + Span nuid = stackalloc char[22]; + + // Act + bool result = NuidWriter.TryWriteNuid(nuid); + + // Assert + Assert.True(result); + string resultAsString = new(nuid); + Assert.Matches("[A-z0-9]{22}", resultAsString); + } + + [Fact] + public void GetNextNuid_PrefixRenewed_Char() + { + bool result = false; + char[] firstNuid = new char[22]; + char[] secondNuid = new char[22]; + + Thread executionThread = new Thread(() => + { + uint increment = 100U; + ulong maxSequential = 839299365868340224ul - increment - 1; + SetSequentialAndIncrement(maxSequential, increment); + + result = NuidWriter.TryWriteNuid(firstNuid); + result &= NuidWriter.TryWriteNuid(secondNuid); + }); + + executionThread.Start(); + executionThread.Join(1_000); + + + // Assert + Assert.True(result); + Assert.False(firstNuid.AsSpan(0, 12).SequenceEqual(secondNuid.AsSpan(0, 12))); + } + + [Fact] + public void GetPrefix_PrefixAsExpected() + { + // Arrange + byte[] rngBytes = new byte[12] { 0, 1, 2, 3, 4, 5, 6, 7, 11, 253, 254, 255 }; + DeterministicRng rng = new(new Queue(new[] { rngBytes, rngBytes })); + + MethodInfo mi = typeof(NuidWriter).GetMethod("GetPrefix", BindingFlags.Static | BindingFlags.NonPublic); + Func mGetPrefix = mi!.CreateDelegate>(); + + // Act + char[] prefix = mGetPrefix(rng); + + // Assert + Assert.Equal(12, prefix.Length); + Assert.True("01234567B567".AsSpan().SequenceEqual(prefix)); + } + + [Fact] + public void InitAndWrite_Char() + { + bool completedSuccessfully = false; + Thread t = new(() => + { + char[] buffer = new char[22]; + bool didWrite = NuidWriter.TryWriteNuid(buffer); + + bool isMatch = _nuidRegex.IsMatch(new string(buffer)); + Volatile.Write(ref completedSuccessfully, didWrite && isMatch); + }); + t.Start(); + t.Join(1_000); + + Assert.True(completedSuccessfully); + } + + [Fact] + public void DifferentThreads_DifferentPrefixes() + { + // Arrange + ConcurrentQueue<(char[] nuid, int threadId)> nuids = new(); + + // Act + for (int i = 0; i < 10; i++) + { + Thread t = new(() => + { + char[] buffer = new char[22]; + NuidWriter.TryWriteNuid(buffer); + nuids.Enqueue((buffer, Environment.CurrentManagedThreadId)); + }); + t.Start(); + t.Join(1_000); + + } + + // Assert + HashSet uniquePrefixes = new HashSet(); + HashSet uniqueThreadIds = new HashSet(); + + foreach ((char[] nuid, int threadId) in nuids.ToList()) + { + string prefix = new string(nuid.AsSpan(0, NuidWriter.PrefixLength)); + Assert.True(uniquePrefixes.Add(prefix), $"Unique prefix {prefix}"); + Assert.True(uniqueThreadIds.Add(threadId), $"Unique thread id {threadId}"); + } + + Assert.Equal(10, uniquePrefixes.Count); + Assert.Equal(10, uniqueThreadIds.Count); + } + + [Fact] + [Trait("Category", "long-running")] + public void AllNuidsAreUnique() + { + const int count = 1_000 * 1_000 * 10; + HashSet nuids = new HashSet(count); + + char[] buffer = new char[22]; + + for (int i = 0; i < count; i++) + { + bool didWrite = NuidWriter.TryWriteNuid(buffer); + + if (!didWrite) + { + Assert.Fail($"Failed to write Nuid, i: {i}"); + } + + string nuid = new(buffer); + + if (!nuids.Add(nuid)) + { + Assert.Fail($"Duplicate Nuid: {nuid} i: {i}"); + } + } + } + + [Fact] + [Trait("Category", "long-running")] + public void AllNuidsAreUnique_SmallSequentials() + { + bool writeFailed = false; + string duplicateFailure = ""; + Thread executionThread = new Thread(() => + { + Span buffer = new char[22]; + for (uint seq = 0; seq < 128; seq++) + { + for (uint incr = 33; incr <= 333; incr++) + { + HashSet nuids = new(2048); + SetSequentialAndIncrement(seq, incr); + + for (int i = 0; i < 2048; i++) + { + if (!NuidWriter.TryWriteNuid(buffer)) + { + writeFailed = true; + return; + } + + //_outputHelper.WriteLine(buffer.ToString()); + + string nuid = new string(buffer); + + if (!nuids.Add(nuid)) + { + duplicateFailure = $"Duplicate nuid: {nuid} seq: {seq} incr: {incr} i: {i}"; + } + } + } + } + }); + + executionThread.Start(); + executionThread.Join(60_000); + + Interlocked.MemoryBarrier(); + + Assert.False(writeFailed); + Assert.Equal("", duplicateFailure); + } + + [Fact] + [Trait("Category", "long-running")] + public void AllNuidsAreUnique_ZeroSequential() + { + bool writeFailed = false; + string duplicateFailure = ""; + Thread executionThread = new Thread(() => + { + uint seq = 0; + uint incr = 33; + + HashSet nuids = new(2048); + SetSequentialAndIncrement(seq, incr); + + Span buffer = new char[22]; + for (int i = 0; i < 100_000_000; i++) + { + if (!NuidWriter.TryWriteNuid(buffer)) + { + writeFailed = true; + return; + } + + //_outputHelper.WriteLine(buffer.ToString()); + + string nuid = new string(buffer); + + if (!nuids.Add(nuid)) + { + duplicateFailure = $"Duplicate nuid: {nuid} seq: {seq} incr: {incr} i: {i}"; + } + } + }); + + executionThread.Start(); + executionThread.Join(120_000); + + Interlocked.MemoryBarrier(); + + Assert.False(writeFailed); + Assert.Equal("", duplicateFailure); + } + + // This messes with NuidWriter's internal state and must be used + // on separate threads (distinct NuidWriter instances) only. + private static void SetSequentialAndIncrement(ulong sequential, ulong increment) + { + bool didWrite = NuidWriter.TryWriteNuid(new char[128]); + + Assert.True(didWrite, "didWrite"); + + FieldInfo fInstance = typeof(NuidWriter).GetField("t_writer", BindingFlags.Static | BindingFlags.NonPublic); + object instance = fInstance.GetValue(null); + + FieldInfo fSequential = typeof(NuidWriter).GetField("_sequential", BindingFlags.Instance | BindingFlags.NonPublic); + fSequential.SetValue(instance, sequential); + + FieldInfo fIncrement = typeof(NuidWriter).GetField("_increment", BindingFlags.Instance | BindingFlags.NonPublic); + fIncrement.SetValue(instance, increment); + } + + private sealed class DeterministicRng : RandomNumberGenerator + { + public int GetBytesInvocations; + private readonly Queue _bytes; + + public DeterministicRng(Queue bytes) + { + _bytes = bytes; + } + + public override void GetBytes(byte[] buffer) + { + byte[] nextBytes = _bytes.Dequeue(); + if(nextBytes.Length < buffer.Length) + throw new InvalidOperationException($"Lenght of {nameof(buffer)} is {buffer.Length}, length of {nameof(nextBytes)} is {nextBytes.Length}"); + + Array.Copy(nextBytes, buffer, buffer.Length); + GetBytesInvocations++; + } + } +} From 874e582872eea10698d865c1f648ce5a5ee59850 Mon Sep 17 00:00:00 2001 From: jasper-d Date: Fri, 6 Oct 2023 18:27:44 +0200 Subject: [PATCH 2/7] dotnet format --- sandbox/MicroBenchmark/NewInboxBenchmarks.cs | 16 +-- sandbox/MicroBenchmark/Program.cs | 4 - src/NATS.Client.Core/Internal/NuidWriter.cs | 57 +++++---- .../NatsConnection.RequestReply.cs | 12 +- .../NATS.Client.Core.Tests/NuidWriterTests.cs | 110 +++++++++--------- 5 files changed, 95 insertions(+), 104 deletions(-) diff --git a/sandbox/MicroBenchmark/NewInboxBenchmarks.cs b/sandbox/MicroBenchmark/NewInboxBenchmarks.cs index 826ae0fdd..30bef6b24 100644 --- a/sandbox/MicroBenchmark/NewInboxBenchmarks.cs +++ b/sandbox/MicroBenchmark/NewInboxBenchmarks.cs @@ -1,4 +1,4 @@ -using System.Diagnostics; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -20,16 +20,16 @@ namespace MicroBenchmark; [SimpleJob(RuntimeMoniker.Net70, baseline: true)] public class NewInboxBenchmarks { - private char[] buf = new char[32]; + private char[] _buf = new char[32]; - private static readonly NatsOpts s_longPrefixOpt = NatsOpts.Default + private static readonly NatsOpts LongPrefixOpt = NatsOpts.Default with - { - InboxPrefix = "this-is-a-rather-long-prefix-that-we-use-here" - }; + { + InboxPrefix = "this-is-a-rather-long-prefix-that-we-use-here", + }; private static readonly NatsConnection _connectionDefaultPrefix = new(); - private static readonly NatsConnection _connectionLongPrefix = new(s_longPrefixOpt); + private static readonly NatsConnection _connectionLongPrefix = new(LongPrefixOpt); [GlobalSetup] public void Setup() @@ -41,7 +41,7 @@ public void Setup() [SkipLocalsInit] public bool TryWriteNuid() { - return NuidWriter.TryWriteNuid(buf); + return NuidWriter.TryWriteNuid(_buf); } [Benchmark] diff --git a/sandbox/MicroBenchmark/Program.cs b/sandbox/MicroBenchmark/Program.cs index a9806db04..c9a046727 100644 --- a/sandbox/MicroBenchmark/Program.cs +++ b/sandbox/MicroBenchmark/Program.cs @@ -1,7 +1,3 @@ using BenchmarkDotNet.Running; BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args); - - - - diff --git a/src/NATS.Client.Core/Internal/NuidWriter.cs b/src/NATS.Client.Core/Internal/NuidWriter.cs index f802137a4..6a47571e9 100644 --- a/src/NATS.Client.Core/Internal/NuidWriter.cs +++ b/src/NATS.Client.Core/Internal/NuidWriter.cs @@ -10,15 +10,15 @@ namespace NATS.Client.Core.Internal; internal sealed class NuidWriter { private const nuint BASE = 62; - private const ulong MAX_SEQUENTIAL = 839299365868340224; // 62^10 // 0x1000_0000_0000_0000; // 64 ^10 - private const uint PREFIX_LENGTH = 12; - private const nuint SEQUENTIAL_LENGTH = 10; - private const int MIN_INCREMENT = 33; - private const int MAX_INCREMENT = 333; - internal const nuint NUID_LENGTH = PREFIX_LENGTH + SEQUENTIAL_LENGTH; + private const ulong MAXSEQUENTIAL = 839299365868340224; // 62^10 // 0x1000_0000_0000_0000; // 64 ^10 + private const uint PREFIXLENGTH = 12; + private const nuint SEQUENTIALLENGTH = 10; + private const int MININCREMENT = 33; + private const int MAXINCREMENT = 333; + internal const nuint NUIDLENGTH = PREFIXLENGTH + SEQUENTIALLENGTH; [ThreadStatic] - private static NuidWriter? t_writer; + private static NuidWriter? writer; // TODO: Use UTF8 string literal when upgrading to .NET 7+ private static ReadOnlySpan Digits => "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; @@ -27,7 +27,7 @@ internal sealed class NuidWriter private ulong _increment; private ulong _sequential; - internal static int PrefixLength => (int)PREFIX_LENGTH; + internal static int PrefixLength => (int)PREFIXLENGTH; private NuidWriter() { @@ -36,9 +36,9 @@ private NuidWriter() public static bool TryWriteNuid(Span nuidBuffer) { - if(t_writer is not null) + if (writer is not null) { - return t_writer.TryWriteNuidCore(nuidBuffer); + return writer.TryWriteNuidCore(nuidBuffer); } return InitAndWrite(nuidBuffer); @@ -47,15 +47,15 @@ public static bool TryWriteNuid(Span nuidBuffer) [MethodImpl(MethodImplOptions.NoInlining)] private static bool InitAndWrite(Span span) { - t_writer = new NuidWriter(); - return t_writer.TryWriteNuidCore(span); + writer = new NuidWriter(); + return writer.TryWriteNuidCore(span); } private bool TryWriteNuidCore(Span nuidBuffer) { - ulong sequential = _sequential += _increment; + var sequential = _sequential += _increment; - if(sequential < MAX_SEQUENTIAL) + if (sequential < MAXSEQUENTIAL) { return TryWriteNuidCore(nuidBuffer, _prefix, sequential); } @@ -65,26 +65,26 @@ private bool TryWriteNuidCore(Span nuidBuffer) [MethodImpl(MethodImplOptions.NoInlining)] bool RefreshAndWrite(Span buffer) { - char[] prefix = Refresh(out sequential); + var prefix = Refresh(out sequential); return TryWriteNuidCore(buffer, prefix, sequential); } } private static bool TryWriteNuidCore(Span buffer, Span prefix, ulong sequential) { - if ((uint)buffer.Length < NUID_LENGTH || prefix.Length != PREFIX_LENGTH || (uint)prefix.Length > (uint)buffer.Length) + if ((uint)buffer.Length < NUIDLENGTH || prefix.Length != PREFIXLENGTH || (uint)prefix.Length > (uint)buffer.Length) { return false; } - Unsafe.CopyBlockUnaligned(ref Unsafe.As(ref buffer[0]), ref Unsafe.As(ref prefix[0]), PREFIX_LENGTH * sizeof(char)); + Unsafe.CopyBlockUnaligned(ref Unsafe.As(ref buffer[0]), ref Unsafe.As(ref prefix[0]), PREFIXLENGTH * sizeof(char)); // NOTE: We must never write to digitsPtr! - ref char digitsPtr = ref MemoryMarshal.GetReference(Digits); + ref var digitsPtr = ref MemoryMarshal.GetReference(Digits); - for(nuint i = PREFIX_LENGTH; i < NUID_LENGTH; i++) + for (nuint i = PREFIXLENGTH; i < NUIDLENGTH; i++) { - nuint digitIndex = (nuint)(sequential % BASE); + var digitIndex = (nuint)(sequential % BASE); Unsafe.Add(ref buffer[0], i) = Unsafe.Add(ref digitsPtr, digitIndex); sequential /= BASE; } @@ -96,7 +96,7 @@ private static bool TryWriteNuidCore(Span buffer, Span prefix, ulong [MemberNotNull(nameof(_prefix))] private char[] Refresh(out ulong sequential) { - char[] prefix = _prefix = GetPrefix(); + var prefix = _prefix = GetPrefix(); _increment = GetIncrement(); sequential = _sequential = GetSequential(); return prefix; @@ -104,20 +104,20 @@ private char[] Refresh(out ulong sequential) private static uint GetIncrement() { - return (uint)Random.Shared.Next(MIN_INCREMENT, MAX_INCREMENT + 1); + return (uint)Random.Shared.Next(MININCREMENT, MAXINCREMENT + 1); } private static ulong GetSequential() { - return (ulong)Random.Shared.NextInt64(0, (long)MAX_SEQUENTIAL + 1); + return (ulong)Random.Shared.NextInt64(0, (long)MAXSEQUENTIAL + 1); } private static char[] GetPrefix(RandomNumberGenerator? rng = null) { - Span randomBytes = stackalloc byte[(int)PREFIX_LENGTH]; + Span randomBytes = stackalloc byte[(int)PREFIXLENGTH]; // TODO: For .NET 8+, use GetItems for better distribution - if(rng == null) + if (rng == null) { RandomNumberGenerator.Fill(randomBytes); } @@ -126,15 +126,14 @@ private static char[] GetPrefix(RandomNumberGenerator? rng = null) rng.GetBytes(randomBytes); } - char[] newPrefix = new char[PREFIX_LENGTH]; + var newPrefix = new char[PREFIXLENGTH]; - for(int i = 0; i < randomBytes.Length; i++) + for (var i = 0; i < randomBytes.Length; i++) { - int digitIndex = (int)(randomBytes[i] % BASE); + var digitIndex = (int)(randomBytes[i] % BASE); newPrefix[i] = Digits[digitIndex]; } return newPrefix; } } - diff --git a/src/NATS.Client.Core/NatsConnection.RequestReply.cs b/src/NATS.Client.Core/NatsConnection.RequestReply.cs index fdcefdc4f..1aea0aacc 100644 --- a/src/NATS.Client.Core/NatsConnection.RequestReply.cs +++ b/src/NATS.Client.Core/NatsConnection.RequestReply.cs @@ -17,24 +17,24 @@ public partial class NatsConnection private static string NewInbox(ReadOnlySpan prefix) { Span buffer = stackalloc char[64]; - uint separatorLength = prefix.Length > 0 ? 1u : 0u; - uint totalLength = (uint)prefix.Length + (uint)NuidWriter.NUID_LENGTH + separatorLength; + var separatorLength = prefix.Length > 0 ? 1u : 0u; + var totalLength = (uint)prefix.Length + (uint)NuidWriter.NUIDLENGTH + separatorLength; if (totalLength <= buffer.Length) { - buffer = buffer.Slice(0, (int)totalLength); + buffer = buffer.Slice(0, (int)totalLength); } else { buffer = new char[totalLength]; } - uint totalPrefixLength = (uint)prefix.Length + separatorLength; + var totalPrefixLength = (uint)prefix.Length + separatorLength; if ((uint)buffer.Length > totalPrefixLength && (uint)buffer.Length > (uint)prefix.Length) { prefix.CopyTo(buffer); buffer[prefix.Length] = '.'; - Span remaining = buffer.Slice((int)totalPrefixLength); - bool didWrite = NuidWriter.TryWriteNuid(remaining); + var remaining = buffer.Slice((int)totalPrefixLength); + var didWrite = NuidWriter.TryWriteNuid(remaining); Debug.Assert(didWrite, "didWrite"); return new string(buffer); } diff --git a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs index cd8aa426b..20d3eb589 100644 --- a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs +++ b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs @@ -31,8 +31,8 @@ public void NewInbox_NuidAppended(string? prefix) var inbox = sut.InboxPrefix; var newInbox = sut.NewInbox(); - Assert.Matches($"{prefix ?? ""}{(prefix?.Length > 0 ? "." : "")}[A-z0-9]{{22}}", inbox); - Assert.Matches($"{prefix ?? ""}{(prefix?.Length > 0 ? "." : "")}[A-z0-9]{{22}}.[A-z0-9]{{22}}", newInbox); + Assert.Matches($"{prefix ?? string.Empty}{(prefix?.Length > 0 ? "." : string.Empty)}[A-z0-9]{{22}}", inbox); + Assert.Matches($"{prefix ?? string.Empty}{(prefix?.Length > 0 ? "." : string.Empty)}[A-z0-9]{{22}}.[A-z0-9]{{22}}", newInbox); _outputHelper.WriteLine($"Prefix: '{prefix}'"); _outputHelper.WriteLine($"Inbox: '{inbox}'"); _outputHelper.WriteLine($"NewInbox: '{newInbox}'"); @@ -45,7 +45,7 @@ public void GetNextNuid_ReturnsNuidOfLength22_Char() Span buffer = stackalloc char[44]; // Act - bool result = NuidWriter.TryWriteNuid(buffer); + var result = NuidWriter.TryWriteNuid(buffer); // Assert ReadOnlySpan lower = buffer.Slice(0, 22); @@ -62,10 +62,10 @@ public void GetNextNuid_ReturnsNuidOfLength22_Char() public void GetNextNuid_BufferToShort_False_Char() { // Arrange - Span nuid = stackalloc char[(int)NuidWriter.NUID_LENGTH - 1]; + Span nuid = stackalloc char[(int)NuidWriter.NUIDLENGTH - 1]; // Act - bool result = NuidWriter.TryWriteNuid(nuid); + var result = NuidWriter.TryWriteNuid(nuid); // Assert Assert.False(result); @@ -80,7 +80,7 @@ public void GetNextNuid_ReturnsDifferentNuidEachTime_Char() Span secondNuid = stackalloc char[22]; // Act - bool result = NuidWriter.TryWriteNuid(firstNuid); + var result = NuidWriter.TryWriteNuid(firstNuid); result &= NuidWriter.TryWriteNuid(secondNuid); // Assert @@ -96,7 +96,7 @@ public void GetNextNuid_PrefixIsConstant_Char() Span secondNuid = stackalloc char[22]; // Act - bool result = NuidWriter.TryWriteNuid(firstNuid); + var result = NuidWriter.TryWriteNuid(firstNuid); result &= NuidWriter.TryWriteNuid(secondNuid); // Assert @@ -111,7 +111,7 @@ public void GetNextNuid_ContainsOnlyValidCharacters_Char() Span nuid = stackalloc char[22]; // Act - bool result = NuidWriter.TryWriteNuid(nuid); + var result = NuidWriter.TryWriteNuid(nuid); // Assert Assert.True(result); @@ -122,14 +122,14 @@ public void GetNextNuid_ContainsOnlyValidCharacters_Char() [Fact] public void GetNextNuid_PrefixRenewed_Char() { - bool result = false; - char[] firstNuid = new char[22]; - char[] secondNuid = new char[22]; + var result = false; + var firstNuid = new char[22]; + var secondNuid = new char[22]; - Thread executionThread = new Thread(() => + var executionThread = new Thread(() => { - uint increment = 100U; - ulong maxSequential = 839299365868340224ul - increment - 1; + var increment = 100U; + var maxSequential = 839299365868340224ul - increment - 1; SetSequentialAndIncrement(maxSequential, increment); result = NuidWriter.TryWriteNuid(firstNuid); @@ -139,7 +139,6 @@ public void GetNextNuid_PrefixRenewed_Char() executionThread.Start(); executionThread.Join(1_000); - // Assert Assert.True(result); Assert.False(firstNuid.AsSpan(0, 12).SequenceEqual(secondNuid.AsSpan(0, 12))); @@ -149,14 +148,14 @@ public void GetNextNuid_PrefixRenewed_Char() public void GetPrefix_PrefixAsExpected() { // Arrange - byte[] rngBytes = new byte[12] { 0, 1, 2, 3, 4, 5, 6, 7, 11, 253, 254, 255 }; + var rngBytes = new byte[12] { 0, 1, 2, 3, 4, 5, 6, 7, 11, 253, 254, 255 }; DeterministicRng rng = new(new Queue(new[] { rngBytes, rngBytes })); - MethodInfo mi = typeof(NuidWriter).GetMethod("GetPrefix", BindingFlags.Static | BindingFlags.NonPublic); - Func mGetPrefix = mi!.CreateDelegate>(); + var mi = typeof(NuidWriter).GetMethod("GetPrefix", BindingFlags.Static | BindingFlags.NonPublic); + var mGetPrefix = mi!.CreateDelegate>(); // Act - char[] prefix = mGetPrefix(rng); + var prefix = mGetPrefix(rng); // Assert Assert.Equal(12, prefix.Length); @@ -166,13 +165,13 @@ public void GetPrefix_PrefixAsExpected() [Fact] public void InitAndWrite_Char() { - bool completedSuccessfully = false; + var completedSuccessfully = false; Thread t = new(() => { - char[] buffer = new char[22]; - bool didWrite = NuidWriter.TryWriteNuid(buffer); + var buffer = new char[22]; + var didWrite = NuidWriter.TryWriteNuid(buffer); - bool isMatch = _nuidRegex.IsMatch(new string(buffer)); + var isMatch = _nuidRegex.IsMatch(new string(buffer)); Volatile.Write(ref completedSuccessfully, didWrite && isMatch); }); t.Start(); @@ -188,26 +187,25 @@ public void DifferentThreads_DifferentPrefixes() ConcurrentQueue<(char[] nuid, int threadId)> nuids = new(); // Act - for (int i = 0; i < 10; i++) + for (var i = 0; i < 10; i++) { Thread t = new(() => { - char[] buffer = new char[22]; + var buffer = new char[22]; NuidWriter.TryWriteNuid(buffer); nuids.Enqueue((buffer, Environment.CurrentManagedThreadId)); }); t.Start(); t.Join(1_000); - } // Assert - HashSet uniquePrefixes = new HashSet(); - HashSet uniqueThreadIds = new HashSet(); + var uniquePrefixes = new HashSet(); + var uniqueThreadIds = new HashSet(); - foreach ((char[] nuid, int threadId) in nuids.ToList()) + foreach ((var nuid, var threadId) in nuids.ToList()) { - string prefix = new string(nuid.AsSpan(0, NuidWriter.PrefixLength)); + var prefix = new string(nuid.AsSpan(0, NuidWriter.PrefixLength)); Assert.True(uniquePrefixes.Add(prefix), $"Unique prefix {prefix}"); Assert.True(uniqueThreadIds.Add(threadId), $"Unique thread id {threadId}"); } @@ -221,13 +219,13 @@ public void DifferentThreads_DifferentPrefixes() public void AllNuidsAreUnique() { const int count = 1_000 * 1_000 * 10; - HashSet nuids = new HashSet(count); + var nuids = new HashSet(count); - char[] buffer = new char[22]; + var buffer = new char[22]; - for (int i = 0; i < count; i++) + for (var i = 0; i < count; i++) { - bool didWrite = NuidWriter.TryWriteNuid(buffer); + var didWrite = NuidWriter.TryWriteNuid(buffer); if (!didWrite) { @@ -247,9 +245,9 @@ public void AllNuidsAreUnique() [Trait("Category", "long-running")] public void AllNuidsAreUnique_SmallSequentials() { - bool writeFailed = false; - string duplicateFailure = ""; - Thread executionThread = new Thread(() => + var writeFailed = false; + var duplicateFailure = string.Empty; + var executionThread = new Thread(() => { Span buffer = new char[22]; for (uint seq = 0; seq < 128; seq++) @@ -259,7 +257,7 @@ public void AllNuidsAreUnique_SmallSequentials() HashSet nuids = new(2048); SetSequentialAndIncrement(seq, incr); - for (int i = 0; i < 2048; i++) + for (var i = 0; i < 2048; i++) { if (!NuidWriter.TryWriteNuid(buffer)) { @@ -267,9 +265,8 @@ public void AllNuidsAreUnique_SmallSequentials() return; } - //_outputHelper.WriteLine(buffer.ToString()); - - string nuid = new string(buffer); + // _outputHelper.WriteLine(buffer.ToString()); + var nuid = new string(buffer); if (!nuids.Add(nuid)) { @@ -286,16 +283,16 @@ public void AllNuidsAreUnique_SmallSequentials() Interlocked.MemoryBarrier(); Assert.False(writeFailed); - Assert.Equal("", duplicateFailure); + Assert.Equal(string.Empty, duplicateFailure); } [Fact] [Trait("Category", "long-running")] public void AllNuidsAreUnique_ZeroSequential() { - bool writeFailed = false; - string duplicateFailure = ""; - Thread executionThread = new Thread(() => + var writeFailed = false; + var duplicateFailure = string.Empty; + var executionThread = new Thread(() => { uint seq = 0; uint incr = 33; @@ -304,7 +301,7 @@ public void AllNuidsAreUnique_ZeroSequential() SetSequentialAndIncrement(seq, incr); Span buffer = new char[22]; - for (int i = 0; i < 100_000_000; i++) + for (var i = 0; i < 100_000_000; i++) { if (!NuidWriter.TryWriteNuid(buffer)) { @@ -312,9 +309,8 @@ public void AllNuidsAreUnique_ZeroSequential() return; } - //_outputHelper.WriteLine(buffer.ToString()); - - string nuid = new string(buffer); + // _outputHelper.WriteLine(buffer.ToString()); + var nuid = new string(buffer); if (!nuids.Add(nuid)) { @@ -329,24 +325,24 @@ public void AllNuidsAreUnique_ZeroSequential() Interlocked.MemoryBarrier(); Assert.False(writeFailed); - Assert.Equal("", duplicateFailure); + Assert.Equal(string.Empty, duplicateFailure); } // This messes with NuidWriter's internal state and must be used // on separate threads (distinct NuidWriter instances) only. private static void SetSequentialAndIncrement(ulong sequential, ulong increment) { - bool didWrite = NuidWriter.TryWriteNuid(new char[128]); + var didWrite = NuidWriter.TryWriteNuid(new char[128]); Assert.True(didWrite, "didWrite"); - FieldInfo fInstance = typeof(NuidWriter).GetField("t_writer", BindingFlags.Static | BindingFlags.NonPublic); - object instance = fInstance.GetValue(null); + var fInstance = typeof(NuidWriter).GetField("t_writer", BindingFlags.Static | BindingFlags.NonPublic); + var instance = fInstance.GetValue(null); - FieldInfo fSequential = typeof(NuidWriter).GetField("_sequential", BindingFlags.Instance | BindingFlags.NonPublic); + var fSequential = typeof(NuidWriter).GetField("_sequential", BindingFlags.Instance | BindingFlags.NonPublic); fSequential.SetValue(instance, sequential); - FieldInfo fIncrement = typeof(NuidWriter).GetField("_increment", BindingFlags.Instance | BindingFlags.NonPublic); + var fIncrement = typeof(NuidWriter).GetField("_increment", BindingFlags.Instance | BindingFlags.NonPublic); fIncrement.SetValue(instance, increment); } @@ -362,8 +358,8 @@ public DeterministicRng(Queue bytes) public override void GetBytes(byte[] buffer) { - byte[] nextBytes = _bytes.Dequeue(); - if(nextBytes.Length < buffer.Length) + var nextBytes = _bytes.Dequeue(); + if (nextBytes.Length < buffer.Length) throw new InvalidOperationException($"Lenght of {nameof(buffer)} is {buffer.Length}, length of {nameof(nextBytes)} is {nextBytes.Length}"); Array.Copy(nextBytes, buffer, buffer.Length); From 00d708655ee19e5ce287d845597838150e5906b2 Mon Sep 17 00:00:00 2001 From: jasper-d Date: Fri, 6 Oct 2023 18:37:13 +0200 Subject: [PATCH 3/7] Manually fix style analyzer warnings --- sandbox/MicroBenchmark/NewInboxBenchmarks.cs | 29 ++--- src/NATS.Client.Core/Internal/NuidWriter.cs | 120 +++++++++--------- .../NatsConnection.RequestReply.cs | 73 ++++++----- .../NATS.Client.Core.Tests/NuidWriterTests.cs | 25 ++-- 4 files changed, 115 insertions(+), 132 deletions(-) diff --git a/sandbox/MicroBenchmark/NewInboxBenchmarks.cs b/sandbox/MicroBenchmark/NewInboxBenchmarks.cs index 30bef6b24..e55c47967 100644 --- a/sandbox/MicroBenchmark/NewInboxBenchmarks.cs +++ b/sandbox/MicroBenchmark/NewInboxBenchmarks.cs @@ -1,11 +1,5 @@ -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; -using System.Runtime.Intrinsics; -using System.Security.Cryptography; using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Jobs; using NATS.Client.Core; using NATS.Client.Core.Internal; @@ -13,23 +7,22 @@ namespace MicroBenchmark; [MemoryDiagnoser] - -[SimpleJob(RuntimeMoniker.Net80)] -[SimpleJob(RuntimeMoniker.NativeAot80)] [SimpleJob(RuntimeMoniker.Net60)] [SimpleJob(RuntimeMoniker.Net70, baseline: true)] +[SimpleJob(RuntimeMoniker.Net80)] +[SimpleJob(RuntimeMoniker.NativeAot80)] public class NewInboxBenchmarks { - private char[] _buf = new char[32]; - private static readonly NatsOpts LongPrefixOpt = NatsOpts.Default with - { - InboxPrefix = "this-is-a-rather-long-prefix-that-we-use-here", - }; + { + InboxPrefix = "this-is-a-rather-long-prefix-that-we-use-here", + }; - private static readonly NatsConnection _connectionDefaultPrefix = new(); - private static readonly NatsConnection _connectionLongPrefix = new(LongPrefixOpt); + private static readonly NatsConnection ConnectionDefaultPrefix = new(); + private static readonly NatsConnection ConnectionLongPrefix = new(LongPrefixOpt); + + private char[] _buf = new char[32]; [GlobalSetup] public void Setup() @@ -47,12 +40,12 @@ public bool TryWriteNuid() [Benchmark] public string NewInbox_ShortPrefix() { - return _connectionDefaultPrefix.NewInbox(); + return ConnectionDefaultPrefix.NewInbox(); } [Benchmark] public string NewInbox_LongPrefix() { - return _connectionLongPrefix.NewInbox(); + return ConnectionLongPrefix.NewInbox(); } } diff --git a/src/NATS.Client.Core/Internal/NuidWriter.cs b/src/NATS.Client.Core/Internal/NuidWriter.cs index 6a47571e9..d233d3d17 100644 --- a/src/NATS.Client.Core/Internal/NuidWriter.cs +++ b/src/NATS.Client.Core/Internal/NuidWriter.cs @@ -1,7 +1,6 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Runtime.Intrinsics; using System.Security.Cryptography; namespace NATS.Client.Core.Internal; @@ -9,112 +8,73 @@ namespace NATS.Client.Core.Internal; [SkipLocalsInit] internal sealed class NuidWriter { - private const nuint BASE = 62; - private const ulong MAXSEQUENTIAL = 839299365868340224; // 62^10 // 0x1000_0000_0000_0000; // 64 ^10 - private const uint PREFIXLENGTH = 12; - private const nuint SEQUENTIALLENGTH = 10; - private const int MININCREMENT = 33; - private const int MAXINCREMENT = 333; - internal const nuint NUIDLENGTH = PREFIXLENGTH + SEQUENTIALLENGTH; + internal const nuint NuidLength = PrefixLength + SequentialLength; + private const nuint Base = 62; + private const ulong MaxSequential = 839299365868340224; // 62^10 + private const uint PrefixLength = 12; + private const nuint SequentialLength = 10; + private const int MinIncrement = 33; + private const int MaxIncrement = 333; [ThreadStatic] - private static NuidWriter? writer; - - // TODO: Use UTF8 string literal when upgrading to .NET 7+ - private static ReadOnlySpan Digits => "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + private static NuidWriter? _writer; private char[] _prefix; private ulong _increment; private ulong _sequential; - internal static int PrefixLength => (int)PREFIXLENGTH; - private NuidWriter() { Refresh(out _); } + private static ReadOnlySpan Digits => "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + public static bool TryWriteNuid(Span nuidBuffer) { - if (writer is not null) + if (_writer is not null) { - return writer.TryWriteNuidCore(nuidBuffer); + return _writer.TryWriteNuidCore(nuidBuffer); } return InitAndWrite(nuidBuffer); } - [MethodImpl(MethodImplOptions.NoInlining)] - private static bool InitAndWrite(Span span) - { - writer = new NuidWriter(); - return writer.TryWriteNuidCore(span); - } - - private bool TryWriteNuidCore(Span nuidBuffer) - { - var sequential = _sequential += _increment; - - if (sequential < MAXSEQUENTIAL) - { - return TryWriteNuidCore(nuidBuffer, _prefix, sequential); - } - - return RefreshAndWrite(nuidBuffer); - - [MethodImpl(MethodImplOptions.NoInlining)] - bool RefreshAndWrite(Span buffer) - { - var prefix = Refresh(out sequential); - return TryWriteNuidCore(buffer, prefix, sequential); - } - } - private static bool TryWriteNuidCore(Span buffer, Span prefix, ulong sequential) { - if ((uint)buffer.Length < NUIDLENGTH || prefix.Length != PREFIXLENGTH || (uint)prefix.Length > (uint)buffer.Length) + if ((uint)buffer.Length < NuidLength || prefix.Length != PrefixLength || (uint)prefix.Length > (uint)buffer.Length) { return false; } - Unsafe.CopyBlockUnaligned(ref Unsafe.As(ref buffer[0]), ref Unsafe.As(ref prefix[0]), PREFIXLENGTH * sizeof(char)); + Unsafe.CopyBlockUnaligned(ref Unsafe.As(ref buffer[0]), ref Unsafe.As(ref prefix[0]), PrefixLength * sizeof(char)); // NOTE: We must never write to digitsPtr! ref var digitsPtr = ref MemoryMarshal.GetReference(Digits); - for (nuint i = PREFIXLENGTH; i < NUIDLENGTH; i++) + for (nuint i = PrefixLength; i < NuidLength; i++) { - var digitIndex = (nuint)(sequential % BASE); + var digitIndex = (nuint)(sequential % Base); Unsafe.Add(ref buffer[0], i) = Unsafe.Add(ref digitsPtr, digitIndex); - sequential /= BASE; + sequential /= Base; } return true; } - [MethodImpl(MethodImplOptions.NoInlining)] - [MemberNotNull(nameof(_prefix))] - private char[] Refresh(out ulong sequential) - { - var prefix = _prefix = GetPrefix(); - _increment = GetIncrement(); - sequential = _sequential = GetSequential(); - return prefix; - } - private static uint GetIncrement() { - return (uint)Random.Shared.Next(MININCREMENT, MAXINCREMENT + 1); + return (uint)Random.Shared.Next(MinIncrement, MaxIncrement + 1); } private static ulong GetSequential() { - return (ulong)Random.Shared.NextInt64(0, (long)MAXSEQUENTIAL + 1); + return (ulong)Random.Shared.NextInt64(0, (long)MaxSequential + 1); } private static char[] GetPrefix(RandomNumberGenerator? rng = null) { - Span randomBytes = stackalloc byte[(int)PREFIXLENGTH]; + Span randomBytes = stackalloc byte[(int)PrefixLength]; // TODO: For .NET 8+, use GetItems for better distribution if (rng == null) @@ -126,14 +86,50 @@ private static char[] GetPrefix(RandomNumberGenerator? rng = null) rng.GetBytes(randomBytes); } - var newPrefix = new char[PREFIXLENGTH]; + var newPrefix = new char[PrefixLength]; for (var i = 0; i < randomBytes.Length; i++) { - var digitIndex = (int)(randomBytes[i] % BASE); + var digitIndex = (int)(randomBytes[i] % Base); newPrefix[i] = Digits[digitIndex]; } return newPrefix; } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool InitAndWrite(Span span) + { + _writer = new NuidWriter(); + return _writer.TryWriteNuidCore(span); + } + + private bool TryWriteNuidCore(Span nuidBuffer) + { + var sequential = _sequential += _increment; + + if (sequential < MaxSequential) + { + return TryWriteNuidCore(nuidBuffer, _prefix, sequential); + } + + return RefreshAndWrite(nuidBuffer); + + [MethodImpl(MethodImplOptions.NoInlining)] + bool RefreshAndWrite(Span buffer) + { + var prefix = Refresh(out sequential); + return TryWriteNuidCore(buffer, prefix, sequential); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + [MemberNotNull(nameof(_prefix))] + private char[] Refresh(out ulong sequential) + { + var prefix = _prefix = GetPrefix(); + _increment = GetIncrement(); + sequential = _sequential = GetSequential(); + return prefix; + } } diff --git a/src/NATS.Client.Core/NatsConnection.RequestReply.cs b/src/NATS.Client.Core/NatsConnection.RequestReply.cs index 1aea0aacc..ca7ba7088 100644 --- a/src/NATS.Client.Core/NatsConnection.RequestReply.cs +++ b/src/NATS.Client.Core/NatsConnection.RequestReply.cs @@ -1,4 +1,3 @@ -using System.Buffers; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; @@ -13,42 +12,6 @@ public partial class NatsConnection /// public string NewInbox() => NewInbox(InboxPrefix); - [SkipLocalsInit] - private static string NewInbox(ReadOnlySpan prefix) - { - Span buffer = stackalloc char[64]; - var separatorLength = prefix.Length > 0 ? 1u : 0u; - var totalLength = (uint)prefix.Length + (uint)NuidWriter.NUIDLENGTH + separatorLength; - if (totalLength <= buffer.Length) - { - buffer = buffer.Slice(0, (int)totalLength); - } - else - { - buffer = new char[totalLength]; - } - - var totalPrefixLength = (uint)prefix.Length + separatorLength; - if ((uint)buffer.Length > totalPrefixLength && (uint)buffer.Length > (uint)prefix.Length) - { - prefix.CopyTo(buffer); - buffer[prefix.Length] = '.'; - var remaining = buffer.Slice((int)totalPrefixLength); - var didWrite = NuidWriter.TryWriteNuid(remaining); - Debug.Assert(didWrite, "didWrite"); - return new string(buffer); - } - - return Throw(); - - [DoesNotReturn] - string Throw() - { - Debug.Fail("Must not happen"); - throw new InvalidOperationException("This should never be raised!"); - } - } - /// public async ValueTask?> RequestAsync( string subject, @@ -101,6 +64,42 @@ string Throw() } } + [SkipLocalsInit] + private static string NewInbox(ReadOnlySpan prefix) + { + Span buffer = stackalloc char[64]; + var separatorLength = prefix.Length > 0 ? 1u : 0u; + var totalLength = (uint)prefix.Length + (uint)NuidWriter.NuidLength + separatorLength; + if (totalLength <= buffer.Length) + { + buffer = buffer.Slice(0, (int)totalLength); + } + else + { + buffer = new char[totalLength]; + } + + var totalPrefixLength = (uint)prefix.Length + separatorLength; + if ((uint)buffer.Length > totalPrefixLength && (uint)buffer.Length > (uint)prefix.Length) + { + prefix.CopyTo(buffer); + buffer[prefix.Length] = '.'; + var remaining = buffer.Slice((int)totalPrefixLength); + var didWrite = NuidWriter.TryWriteNuid(remaining); + Debug.Assert(didWrite, "didWrite"); + return new string(buffer); + } + + return Throw(); + + [DoesNotReturn] + string Throw() + { + Debug.Fail("Must not happen"); + throw new InvalidOperationException("This should never be raised!"); + } + } + private NatsSubOpts SetReplyOptsDefaults(NatsSubOpts? replyOpts) { var opts = replyOpts ?? DefaultReplyOpts; diff --git a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs index 20d3eb589..d5e80c963 100644 --- a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs +++ b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs @@ -1,15 +1,13 @@ using System.Collections.Concurrent; -using System.Diagnostics; using System.Reflection; using System.Security.Cryptography; -using System.Text; using System.Text.RegularExpressions; namespace NATS.Client.Core.Tests; public class NuidWriterTests { - private static readonly Regex _nuidRegex = new Regex("[A-z0-9]{22}"); + private static readonly Regex NuidRegex = new("[A-z0-9]{22}"); private readonly ITestOutputHelper _outputHelper; @@ -25,7 +23,7 @@ public NuidWriterTests(ITestOutputHelper outputHelper) [InlineData("long-inbox-prefix-above-stackalloc-limit-of-64")] public void NewInbox_NuidAppended(string? prefix) { - var natsOpts = NatsOpts.Default with { InboxPrefix = prefix }; + var natsOpts = NatsOpts.Default with { InboxPrefix = prefix! }; var sut = new NatsConnection(natsOpts); var inbox = sut.InboxPrefix; @@ -62,7 +60,7 @@ public void GetNextNuid_ReturnsNuidOfLength22_Char() public void GetNextNuid_BufferToShort_False_Char() { // Arrange - Span nuid = stackalloc char[(int)NuidWriter.NUIDLENGTH - 1]; + Span nuid = stackalloc char[(int)NuidWriter.NuidLength - 1]; // Act var result = NuidWriter.TryWriteNuid(nuid); @@ -171,7 +169,7 @@ public void InitAndWrite_Char() var buffer = new char[22]; var didWrite = NuidWriter.TryWriteNuid(buffer); - var isMatch = _nuidRegex.IsMatch(new string(buffer)); + var isMatch = NuidRegex.IsMatch(new string(buffer)); Volatile.Write(ref completedSuccessfully, didWrite && isMatch); }); t.Start(); @@ -184,6 +182,7 @@ public void InitAndWrite_Char() public void DifferentThreads_DifferentPrefixes() { // Arrange + const int prefixLength = 12; ConcurrentQueue<(char[] nuid, int threadId)> nuids = new(); // Act @@ -203,9 +202,9 @@ public void DifferentThreads_DifferentPrefixes() var uniquePrefixes = new HashSet(); var uniqueThreadIds = new HashSet(); - foreach ((var nuid, var threadId) in nuids.ToList()) + foreach (var (nuid, threadId) in nuids.ToList()) { - var prefix = new string(nuid.AsSpan(0, NuidWriter.PrefixLength)); + var prefix = new string(nuid.AsSpan(0, prefixLength)); Assert.True(uniquePrefixes.Add(prefix), $"Unique prefix {prefix}"); Assert.True(uniqueThreadIds.Add(threadId), $"Unique thread id {threadId}"); } @@ -265,7 +264,6 @@ public void AllNuidsAreUnique_SmallSequentials() return; } - // _outputHelper.WriteLine(buffer.ToString()); var nuid = new string(buffer); if (!nuids.Add(nuid)) @@ -309,7 +307,6 @@ public void AllNuidsAreUnique_ZeroSequential() return; } - // _outputHelper.WriteLine(buffer.ToString()); var nuid = new string(buffer); if (!nuids.Add(nuid)) @@ -337,18 +334,17 @@ private static void SetSequentialAndIncrement(ulong sequential, ulong increment) Assert.True(didWrite, "didWrite"); var fInstance = typeof(NuidWriter).GetField("t_writer", BindingFlags.Static | BindingFlags.NonPublic); - var instance = fInstance.GetValue(null); + var instance = fInstance!.GetValue(null); var fSequential = typeof(NuidWriter).GetField("_sequential", BindingFlags.Instance | BindingFlags.NonPublic); - fSequential.SetValue(instance, sequential); + fSequential!.SetValue(instance, sequential); var fIncrement = typeof(NuidWriter).GetField("_increment", BindingFlags.Instance | BindingFlags.NonPublic); - fIncrement.SetValue(instance, increment); + fIncrement!.SetValue(instance, increment); } private sealed class DeterministicRng : RandomNumberGenerator { - public int GetBytesInvocations; private readonly Queue _bytes; public DeterministicRng(Queue bytes) @@ -363,7 +359,6 @@ public override void GetBytes(byte[] buffer) throw new InvalidOperationException($"Lenght of {nameof(buffer)} is {buffer.Length}, length of {nameof(nextBytes)} is {nextBytes.Length}"); Array.Copy(nextBytes, buffer, buffer.Length); - GetBytesInvocations++; } } } From e10dba03b9849a61b06983c671483b57e1990976 Mon Sep 17 00:00:00 2001 From: jasper-d Date: Fri, 6 Oct 2023 20:33:19 +0200 Subject: [PATCH 4/7] Fix tests --- tests/NATS.Client.Core.Tests/NuidWriterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs index d5e80c963..171716df1 100644 --- a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs +++ b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs @@ -333,7 +333,7 @@ private static void SetSequentialAndIncrement(ulong sequential, ulong increment) Assert.True(didWrite, "didWrite"); - var fInstance = typeof(NuidWriter).GetField("t_writer", BindingFlags.Static | BindingFlags.NonPublic); + var fInstance = typeof(NuidWriter).GetField("_writer", BindingFlags.Static | BindingFlags.NonPublic); var instance = fInstance!.GetValue(null); var fSequential = typeof(NuidWriter).GetField("_sequential", BindingFlags.Instance | BindingFlags.NonPublic); From b8c7d4e1ecde90a31bfd882954fd8745a56a0ef7 Mon Sep 17 00:00:00 2001 From: jasper-d Date: Fri, 6 Oct 2023 20:41:45 +0200 Subject: [PATCH 5/7] Skip long running tests --- tests/NATS.Client.Core.Tests/NuidWriterTests.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs index 171716df1..84577d296 100644 --- a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs +++ b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs @@ -213,8 +213,7 @@ public void DifferentThreads_DifferentPrefixes() Assert.Equal(10, uniqueThreadIds.Count); } - [Fact] - [Trait("Category", "long-running")] + [Fact(Skip = "long running")] public void AllNuidsAreUnique() { const int count = 1_000 * 1_000 * 10; @@ -240,8 +239,7 @@ public void AllNuidsAreUnique() } } - [Fact] - [Trait("Category", "long-running")] + [Fact(Skip = "long running")] public void AllNuidsAreUnique_SmallSequentials() { var writeFailed = false; @@ -284,8 +282,7 @@ public void AllNuidsAreUnique_SmallSequentials() Assert.Equal(string.Empty, duplicateFailure); } - [Fact] - [Trait("Category", "long-running")] + [Fact(Skip = "long running")] public void AllNuidsAreUnique_ZeroSequential() { var writeFailed = false; From 045533923691710ffeb0a36971962c91b458850d Mon Sep 17 00:00:00 2001 From: jasper-d Date: Fri, 6 Oct 2023 20:52:12 +0200 Subject: [PATCH 6/7] Fix CI build --- sandbox/MicroBenchmark/MicroBenchmark.csproj | 1 - src/NATS.Client.Core/NATS.Client.Core.csproj | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/sandbox/MicroBenchmark/MicroBenchmark.csproj b/sandbox/MicroBenchmark/MicroBenchmark.csproj index 19703b686..dad46b78e 100644 --- a/sandbox/MicroBenchmark/MicroBenchmark.csproj +++ b/sandbox/MicroBenchmark/MicroBenchmark.csproj @@ -3,7 +3,6 @@ Exe net6.0 - net6.0;net8.0 enable enable false diff --git a/src/NATS.Client.Core/NATS.Client.Core.csproj b/src/NATS.Client.Core/NATS.Client.Core.csproj index 7784f29af..11a06cdd7 100644 --- a/src/NATS.Client.Core/NATS.Client.Core.csproj +++ b/src/NATS.Client.Core/NATS.Client.Core.csproj @@ -1,7 +1,7 @@  - net6.0;net7.0;net8.0 + net6.0 enable enable true From 420a338fa3b554f88ed05c1a7555fc6715edaacc Mon Sep 17 00:00:00 2001 From: jasper-d Date: Fri, 6 Oct 2023 21:00:44 +0200 Subject: [PATCH 7/7] Remove unnecessary compare --- src/NATS.Client.Core/Internal/NuidWriter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NATS.Client.Core/Internal/NuidWriter.cs b/src/NATS.Client.Core/Internal/NuidWriter.cs index d233d3d17..5061c078a 100644 --- a/src/NATS.Client.Core/Internal/NuidWriter.cs +++ b/src/NATS.Client.Core/Internal/NuidWriter.cs @@ -42,7 +42,7 @@ public static bool TryWriteNuid(Span nuidBuffer) private static bool TryWriteNuidCore(Span buffer, Span prefix, ulong sequential) { - if ((uint)buffer.Length < NuidLength || prefix.Length != PrefixLength || (uint)prefix.Length > (uint)buffer.Length) + if ((uint)buffer.Length < NuidLength || prefix.Length != PrefixLength) { return false; }