Fixed MassTransit#6124 - NewId.Next(n) was using the wrong algorithm for Guid generation

Author: phatboyg

src/MassTransit.Abstractions/NewId/INewIdGenerator.cs

@@ -11,6 +11,8 @@ public interface INewIdGenerator
 
         Guid NextGuid();
 
+        ArraySegment<Guid> NextGuid(Guid[] ids, int index, int count);
+
         ArraySegment<Guid> NextSequentialGuid(Guid[] ids, int index, int count);
 
         Guid NextSequentialGuid();

src/MassTransit.Abstractions/NewId/NewId.cs

@@ -6,16 +6,15 @@ namespace MassTransit
     using NewIdFormatters;
     using NewIdProviders;
 #if NET6_0_OR_GREATER
-    using System.Diagnostics;
     using System.Runtime.Intrinsics.X86;
     using System.Runtime.Intrinsics;
     using System.Runtime.InteropServices;
 #endif
 
+    // We need to target netstandard2.0, so keep using ref parameter.
+    // CS9191: The 'ref' modifier for argument 2 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
+    #pragma warning disable CS9191
 
-// We need to target netstandard2.0, so keep using ref parameter.
-// CS9191: The 'ref' modifier for argument 2 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
-#pragma warning disable CS9191
 
     /// <summary>
     /// A NewId is a type that fits into the same space as a Guid/Uuid/unique identifier,
@@ -30,10 +29,10 @@ namespace MassTransit
     {
         public static readonly NewId Empty = new NewId(0, 0, 0, 0);
 
-        static readonly INewIdFormatter BraceFormatter = new DashedHexFormatter('{', '}');
-        static readonly INewIdFormatter DashedHexFormatter = new DashedHexFormatter();
-        static readonly INewIdFormatter HexFormatter = new HexFormatter();
-        static readonly INewIdFormatter ParenFormatter = new DashedHexFormatter('(', ')');
+        static readonly DashedHexFormatter BraceFormatter = new DashedHexFormatter('{', '}');
+        static readonly DashedHexFormatter DashedHexFormatter = new DashedHexFormatter();
+        static readonly HexFormatter HexFormatter = new HexFormatter();
+        static readonly DashedHexFormatter ParenFormatter = new DashedHexFormatter('(', ')');
 
         static INewIdGenerator? _generator;
         static ITickProvider? _tickProvider;
@@ -477,7 +476,7 @@ public static Guid[] NextGuid(int count)
         {
             var ids = new Guid[count];
 
-            _getGenerator().NextSequentialGuid(ids, 0, count);
+            _getGenerator().NextGuid(ids, 0, count);
 
             return ids;
         }
@@ -491,7 +490,7 @@ public static Guid[] NextGuid(int count)
         /// <returns></returns>
         public static ArraySegment<Guid> NextGuid(Guid[] ids, int index, int count)
         {
-            return _getGenerator().NextSequentialGuid(ids, index, count);
+            return _getGenerator().NextGuid(ids, index, count);
         }
 
         /// <summary>
@@ -512,6 +511,30 @@ public static Guid NextSequentialGuid()
             return _getGenerator().NextSequentialGuid();
         }
 
+        /// <summary>
+        /// Generate an array of NewIds
+        /// </summary>
+        /// <param name="count">The number of NewIds to generate</param>
+        /// <returns></returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static Guid[] NextSequentialGuid(int count)
+        {
+            var ids = new Guid[count];
+
+            _getGenerator().NextSequentialGuid(ids, 0, count);
+
+            return ids;
+        }
+
+        /// <summary>
+        /// Generate an array of NewIds, and return it as a Guid in sequential format
+        /// </summary>
+        /// <returns></returns>
+        public static ArraySegment<Guid> NextSequentialGuid(Guid[] ids, int index, int count)
+        {
+            return _getGenerator().NextSequentialGuid(ids, index, count);
+        }
+
     #if NET6_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         static void FromGuid(in Guid guid, out NewId newId)

src/MassTransit.Abstractions/NewId/NewIdGenerator.cs

@@ -92,14 +92,14 @@ public Guid NextGuid()
             // swapping high and low byte, because SQL-server is doing the wrong ordering otherwise
             var sequenceSwapped = ((sequence << 8) | ((sequence >> 8) & 0x00FF)) & 0xFFFF;
 
-#if NET6_0_OR_GREATER
+        #if NET6_0_OR_GREATER
             if (Ssse3.IsSupported && BitConverter.IsLittleEndian)
             {
                 var vec = Vector128.Create((int)a, b, _c, _d | sequenceSwapped);
                 var result = Ssse3.Shuffle(vec.AsByte(), Vector128.Create((byte)12, 13, 14, 15, 8, 9, 10, 11, 5, 4, 3, 2, 1, 0, 7, 6));
                 return Unsafe.As<Vector128<byte>, Guid>(ref result);
             }
-#endif
+        #endif
 
             var d = (byte)(b >> 8);
             var e = (byte)b;
@@ -128,9 +128,9 @@ public Guid NextSequentialGuid()
             var sequence = _sequence++;
 
             var a = _a;
-#if NET6_0_OR_GREATER
+        #if NET6_0_OR_GREATER
             var v = _b;
-#endif
+        #endif
             var b = (short)(_b >> 16);
             var c = (short)_b;
 
@@ -140,14 +140,14 @@ public Guid NextSequentialGuid()
             // swapping high and low byte, because SQL-server is doing the wrong ordering otherwise
             var sequenceSwapped = ((sequence << 8) | ((sequence >> 8) & 0x00FF)) & 0xFFFF;
 
-#if NET6_0_OR_GREATER
+        #if NET6_0_OR_GREATER
             if (Ssse3.IsSupported && BitConverter.IsLittleEndian)
             {
                 var vec = Vector128.Create((int)a, v, _c, _d | sequenceSwapped);
                 var result = Ssse3.Shuffle(vec.AsByte(), Vector128.Create((byte)0, 1, 2, 3, 6, 7, 4, 5, 11, 10, 9, 8, 15, 14, 13, 12));
                 return Unsafe.As<Vector128<byte>, Guid>(ref result);
             }
-#endif
+        #endif
 
             var d = (byte)(_gc >> 8);
             var e = (byte)_gc;
@@ -190,16 +190,91 @@ public ArraySegment<NewId> Next(NewId[] ids, int index, int count)
             return new ArraySegment<NewId>(ids, index, count);
         }
 
+        public ArraySegment<Guid> NextGuid(Guid[] ids, int index, int count)
+        {
+            if (index + count > ids.Length)
+                throw new ArgumentOutOfRangeException(nameof(count));
+
+            var ticks = _tickProvider.Ticks;
+
+            var a = _a;
+            var b = _b;
+
+            var lockTaken = false;
+            _spinLock.Enter(ref lockTaken);
+
+            if (ticks > _lastTick)
+                UpdateTimestamp(ticks);
+
+            var d = (byte)(b >> 8);
+            var e = (byte)b;
+            var f = (byte)(a >> 24);
+            var g = (byte)(a >> 16);
+            var h = (byte)(a >> 8);
+            var i = (byte)a;
+            var j = (byte)(b >> 24);
+            var k = (byte)(b >> 16);
+
+            var limit = index + count;
+            for (var offset = index; offset < limit; offset++)
+            {
+                if (_sequence == 65535) // we are about to rollover, so we need to increment ticks
+                {
+                    UpdateTimestamp(_lastTick + 1);
+
+                    if (a != _a)
+                    {
+                        a = _a;
+                        f = (byte)(a >> 24);
+                        g = (byte)(a >> 16);
+                        h = (byte)(a >> 8);
+                        i = (byte)a;
+                    }
+
+                    if (b != _b)
+                    {
+                        b = _b;
+                        d = (byte)(b >> 8);
+                        e = (byte)b;
+                        j = (byte)(b >> 24);
+                        k = (byte)(b >> 16);
+                    }
+                }
+
+                var sequence = _sequence++;
+
+                // swapping high and low byte, because SQL-server is doing the wrong ordering otherwise
+                var sequenceSwapped = ((sequence << 8) | ((sequence >> 8) & 0x00FF)) & 0xFFFF;
+
+            #if NET6_0_OR_GREATER
+                if (Ssse3.IsSupported && BitConverter.IsLittleEndian)
+                {
+                    var vec = Vector128.Create((int)a, b, _c, _d | sequenceSwapped);
+                    var result = Ssse3.Shuffle(vec.AsByte(), Vector128.Create((byte)12, 13, 14, 15, 8, 9, 10, 11, 5, 4, 3, 2, 1, 0, 7, 6));
+                    ids[offset] = Unsafe.As<Vector128<byte>, Guid>(ref result);
+                    continue;
+                }
+            #endif
+
+                ids[offset] = new Guid(_d | sequenceSwapped, _gb, _gc, d, e, f, g, h, i, j, k);
+            }
+
+            if (lockTaken)
+                _spinLock.Exit();
+
+            return new ArraySegment<Guid>(ids, index, count);
+        }
+
         public ArraySegment<Guid> NextSequentialGuid(Guid[] ids, int index, int count)
         {
             if (index + count > ids.Length)
                 throw new ArgumentOutOfRangeException(nameof(count));
 
             var ticks = _tickProvider.Ticks;
 
-#if NET6_0_OR_GREATER
+        #if NET6_0_OR_GREATER
             var v = _b;
-#endif
+        #endif
             var a = _a;
             var b = (short)(_b >> 16);
             var c = (short)_b;
@@ -222,9 +297,9 @@ public ArraySegment<Guid> NextSequentialGuid(Guid[] ids, int index, int count)
                 {
                     UpdateTimestamp(_lastTick + 1);
 
-#if NET6_0_OR_GREATER
+                #if NET6_0_OR_GREATER
                     v = _b;
-#endif
+                #endif
                     a = _a;
                     b = (short)(_b >> 16);
                     c = (short)_b;
@@ -235,15 +310,15 @@ public ArraySegment<Guid> NextSequentialGuid(Guid[] ids, int index, int count)
                 // swapping high and low byte, because SQL-server is doing the wrong ordering otherwise
                 var sequenceSwapped = ((sequence << 8) | ((sequence >> 8) & 0x00FF)) & 0xFFFF;
 
-#if NET6_0_OR_GREATER
+            #if NET6_0_OR_GREATER
                 if (Ssse3.IsSupported && BitConverter.IsLittleEndian)
                 {
                     var vec = Vector128.Create((int)a, v, _c, _d | sequenceSwapped);
                     var result = Ssse3.Shuffle(vec.AsByte(), Vector128.Create((byte)0, 1, 2, 3, 6, 7, 4, 5, 11, 10, 9, 8, 15, 14, 13, 12));
                     ids[offset] = Unsafe.As<Vector128<byte>, Guid>(ref result);
                     continue;
                 }
-#endif
+            #endif
 
                 var h = (byte)((_d | sequenceSwapped) >> 24);
                 var i = (byte)((_d | sequenceSwapped) >> 16);

tests/MassTransit.Abstractions.Tests/NewId/NewId_Specs.cs

@@ -166,6 +166,38 @@ public void Should_generate_sequential_ids_quickly()
             }
         }
 
+        [Test]
+        public void Should_be_using_the_correct_algorithm()
+        {
+            NewId.SetTickProvider(new StopwatchTickProvider());
+
+            var first = NewId.NextGuid();
+            Guid[] next = NewId.NextGuid(3);
+
+            for (var i = 0; i < next.Length - 1; i++)
+            {
+                Assert.That(next[i].ToString().Substring(0, 4), Is.EqualTo(first.ToString().Substring(0, 4)));
+                Assert.That(next[i].ToString().Substring(6), Is.EqualTo(first.ToString().Substring(6)));
+                Assert.That(int.Parse(next[i].ToString().Substring(4, 2)), Is.EqualTo(i));
+            }
+        }
+
+        [Test]
+        public void Should_be_using_the_correct_algorithm_for_sequential_guids()
+        {
+            NewId.SetTickProvider(new StopwatchTickProvider());
+
+            var first = NewId.NextSequentialGuid();
+            var next = new Guid[3];
+            NewId.NextSequentialGuid(next, 0, 3);
+
+            for (var i = 0; i < next.Length - 1; i++)
+            {
+                Assert.That(next[i].ToString().Substring(0,32), Is.EqualTo(first.ToString().Substring(0,32)));
+                Assert.That(int.Parse(next[i].ToString().Substring(32,2)), Is.EqualTo(i));
+            }
+        }
+
         [Test]
         [Explicit]
         public void Should_generate_unique_identifiers_with_each_invocation()