From a7d24f54cb5310a37b06a0ddf57ff849450f1c47 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 12 Jan 2021 14:53:19 -0800 Subject: [PATCH 1/8] Optimize Activity Ids --- .../src/System/Diagnostics/Activity.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index a1c6830a77e4c..722c4014a50a1 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -39,6 +39,8 @@ public partial class Activity : IDisposable private static readonly IEnumerable s_emptyLinks = new ActivityLink[0]; private static readonly IEnumerable s_emptyEvents = new ActivityEvent[0]; #pragma warning restore CA1825 + internal static readonly Random s_random = new Random(); + private static readonly ActivitySource s_defaultSource = new ActivitySource(string.Empty); private const byte ActivityTraceFlagsIsSet = 0b_1_0000000; // Internal flag to indicate if flags have been set @@ -1657,15 +1659,22 @@ public void CopyTo(Span destination) } /// - /// Sets the bytes in 'outBytes' to be random values. outBytes.Length must be less than or equal to 16 + /// Sets the bytes in 'outBytes' to be random values. outBytes.Length must be either 8 or 16 bytes. /// /// internal static unsafe void SetToRandomBytes(Span outBytes) { - Debug.Assert(outBytes.Length <= sizeof(Guid)); // Guid is 16 bytes, and so is TraceId - Guid guid = Guid.NewGuid(); - ReadOnlySpan guidBytes = new ReadOnlySpan(&guid, sizeof(Guid)); - guidBytes.Slice(0, outBytes.Length).CopyTo(outBytes); + Debug.Assert(outBytes.Length == 16 || outBytes.Length == 8); + Random r = Activity.s_random; + + Unsafe.WriteUnaligned(ref outBytes[0], r.Next()); + Unsafe.WriteUnaligned(ref outBytes[4], r.Next()); + + if (outBytes.Length >= 16) + { + Unsafe.WriteUnaligned(ref outBytes[8], r.Next()); + Unsafe.WriteUnaligned(ref outBytes[12], r.Next()); + } } /// From f56fe491785d48dffe5aea82043a112bb473d228 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 14 Jan 2021 14:27:08 -0800 Subject: [PATCH 2/8] Generate long random numbers and make the generator object a threadstatic --- ...System.Diagnostics.DiagnosticSource.csproj | 4 +- .../src/System/Diagnostics/Activity.cs | 6 +- .../Diagnostics/RandomNumberGenerator.cs | 115 ++++++++++++++++++ 3 files changed, 118 insertions(+), 7 deletions(-) create mode 100755 src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj index f3a2c373d1872..fa0c354fdbcbc 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj @@ -32,8 +32,7 @@ - + @@ -45,6 +44,7 @@ + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index 722c4014a50a1..1dc871e8fdaff 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -39,8 +39,6 @@ public partial class Activity : IDisposable private static readonly IEnumerable s_emptyLinks = new ActivityLink[0]; private static readonly IEnumerable s_emptyEvents = new ActivityEvent[0]; #pragma warning restore CA1825 - internal static readonly Random s_random = new Random(); - private static readonly ActivitySource s_defaultSource = new ActivitySource(string.Empty); private const byte ActivityTraceFlagsIsSet = 0b_1_0000000; // Internal flag to indicate if flags have been set @@ -1665,15 +1663,13 @@ public void CopyTo(Span destination) internal static unsafe void SetToRandomBytes(Span outBytes) { Debug.Assert(outBytes.Length == 16 || outBytes.Length == 8); - Random r = Activity.s_random; + RandomNumberGenerator r = RandomNumberGenerator.Current; Unsafe.WriteUnaligned(ref outBytes[0], r.Next()); - Unsafe.WriteUnaligned(ref outBytes[4], r.Next()); if (outBytes.Length >= 16) { Unsafe.WriteUnaligned(ref outBytes[8], r.Next()); - Unsafe.WriteUnaligned(ref outBytes[12], r.Next()); } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs new file mode 100755 index 0000000000000..a6c0b96a27b0d --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs @@ -0,0 +1,115 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics +{ + /// + /// RandomNumberGenerator implementation is borrowed from the Random class which implement the Additive Number Generator algorithm for generating random numbers. + /// The difference here is RandomNumberGenerator based on long numbers instead of int numbers + /// to allow generating long numbers and increase the period (which is when the generated number can repeat again) + /// + internal class RandomNumberGenerator + { + private readonly long[] _seedArray = new long[56]; + private int _inext; + private int _inextp; + + [ThreadStatic] private static RandomNumberGenerator? t_random; + + public static RandomNumberGenerator Current + { + get + { + if (t_random == null) + { + t_random = new RandomNumberGenerator(); + } + return t_random; + } + } + + public RandomNumberGenerator() : this(((long)Guid.NewGuid().GetHashCode() << 32) | (long)Guid.NewGuid().GetHashCode()) { } + + public RandomNumberGenerator(long Seed) + { + long subtraction = (Seed == long.MinValue) ? long.MaxValue : Math.Abs(Seed); + long mj = 1618033988749894848L - subtraction; // magic number based on Phi (golden ratio) + _seedArray[55] = mj; + long mk = 1; + + int ii = 0; + for (int i = 1; i < 55; i++) + { + // The range [1..55] is special (Knuth) and so we're wasting the 0'th position. + if ((ii += 21) >= 55) + { + ii -= 55; + } + + _seedArray[ii] = mk; + mk = mj - mk; + if (mk < 0) + { + mk += int.MaxValue; + } + + mj = _seedArray[ii]; + } + + for (int k = 1; k < 5; k++) + { + for (int i = 1; i < 56; i++) + { + int n = i + 30; + if (n >= 55) + { + n -= 55; + } + + _seedArray[i] -= _seedArray[1 + n]; + if (_seedArray[i] < 0) + { + _seedArray[i] += int.MaxValue; + } + } + } + + _inextp = 21; + } + + public long Next() => InternalSample(); + + private long InternalSample() + { + int locINext = _inext; + if (++locINext >= 56) + { + locINext = 1; + } + + int locINextp = _inextp; + if (++locINextp >= 56) + { + locINextp = 1; + } + + long retVal = _seedArray[locINext] - _seedArray[locINextp]; + + if (retVal == long.MaxValue) + { + retVal--; + } + + if (retVal < 0) + { + retVal += long.MaxValue; + } + + _seedArray[locINext] = retVal; + _inext = locINext; + _inextp = locINextp; + + return retVal; + } + } +} \ No newline at end of file From 9dcd750dfff13a22bf0ed75ae15260f86b1ae9ce Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 14 Jan 2021 14:43:04 -0800 Subject: [PATCH 3/8] Addnew line at the end of the file --- .../src/System/Diagnostics/RandomNumberGenerator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs index a6c0b96a27b0d..6289c9c72e591 100755 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs @@ -112,4 +112,4 @@ private long InternalSample() return retVal; } } -} \ No newline at end of file +} From 7a8272f99e6865bac9fb209c3194d531db69893b Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Mon, 18 Jan 2021 13:24:41 -0800 Subject: [PATCH 4/8] Update the Random number generator --- .../Diagnostics/RandomNumberGenerator.cs | 104 +++++------------- 1 file changed, 25 insertions(+), 79 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs index 6289c9c72e591..a1d53684b6e65 100755 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs @@ -4,18 +4,14 @@ namespace System.Diagnostics { /// - /// RandomNumberGenerator implementation is borrowed from the Random class which implement the Additive Number Generator algorithm for generating random numbers. - /// The difference here is RandomNumberGenerator based on long numbers instead of int numbers - /// to allow generating long numbers and increase the period (which is when the generated number can repeat again) + /// RandomNumberGenerator implementation is the 64-bit random number generator based on the Xorshift algorithm (known as shift-register generators). /// internal class RandomNumberGenerator { - private readonly long[] _seedArray = new long[56]; - private int _inext; - private int _inextp; - [ThreadStatic] private static RandomNumberGenerator? t_random; + private ulong _s0, _s1, _s2, _s3; + public static RandomNumberGenerator Current { get @@ -28,88 +24,38 @@ public static RandomNumberGenerator Current } } - public RandomNumberGenerator() : this(((long)Guid.NewGuid().GetHashCode() << 32) | (long)Guid.NewGuid().GetHashCode()) { } - - public RandomNumberGenerator(long Seed) + public unsafe RandomNumberGenerator() { - long subtraction = (Seed == long.MinValue) ? long.MaxValue : Math.Abs(Seed); - long mj = 1618033988749894848L - subtraction; // magic number based on Phi (golden ratio) - _seedArray[55] = mj; - long mk = 1; - - int ii = 0; - for (int i = 1; i < 55; i++) - { - // The range [1..55] is special (Knuth) and so we're wasting the 0'th position. - if ((ii += 21) >= 55) - { - ii -= 55; - } - - _seedArray[ii] = mk; - mk = mj - mk; - if (mk < 0) - { - mk += int.MaxValue; - } - - mj = _seedArray[ii]; - } - - for (int k = 1; k < 5; k++) + do { - for (int i = 1; i < 56; i++) - { - int n = i + 30; - if (n >= 55) - { - n -= 55; - } - - _seedArray[i] -= _seedArray[1 + n]; - if (_seedArray[i] < 0) - { - _seedArray[i] += int.MaxValue; - } - } + Guid g1 = Guid.NewGuid(); + Guid g2 = Guid.NewGuid(); + ulong* g1p = (ulong*)&g1; + ulong* g2p = (ulong*)&g2; + _s0 = *g1p; + _s1 = *(g1p + 1); + _s2 = *g2p; + _s3 = *(g2p + 1); } - - _inextp = 21; + while ((_s0 | _s1 | _s2 | _s3) == 0); } - public long Next() => InternalSample(); + private ulong Rol64(ulong x, int k) => (x << k) | (x >> (64 - k)); - private long InternalSample() + public long Next() { - int locINext = _inext; - if (++locINext >= 56) - { - locINext = 1; - } - - int locINextp = _inextp; - if (++locINextp >= 56) - { - locINextp = 1; - } - - long retVal = _seedArray[locINext] - _seedArray[locINextp]; + ulong result = Rol64(_s1 * 5, 7) * 9; + ulong t = _s1 << 17; - if (retVal == long.MaxValue) - { - retVal--; - } - - if (retVal < 0) - { - retVal += long.MaxValue; - } + _s2 ^= _s0; + _s3 ^= _s1; + _s1 ^= _s2; + _s0 ^= _s3; - _seedArray[locINext] = retVal; - _inext = locINext; - _inextp = locINextp; + _s2 ^= t; + _s3 = Rol64(_s3, 45); - return retVal; + return (long)result; } } } From 69cba20c36bf181506ec279f7e821ecc82d42ad3 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 19 Jan 2021 12:05:07 -0800 Subject: [PATCH 5/8] Fix Full Framework failure --- .../src/System/Diagnostics/RandomNumberGenerator.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs index a1d53684b6e65..a080c930b5b3c 100755 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs @@ -24,6 +24,9 @@ public static RandomNumberGenerator Current } } +#if ALLOW_PARTIALLY_TRUSTED_CALLERS + [System.Security.SecuritySafeCriticalAttribute] +#endif public unsafe RandomNumberGenerator() { do From 9fdac0bed4c46fc94f9d01078706646a1e0e9850 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 20 Jan 2021 14:40:10 -0800 Subject: [PATCH 6/8] Address the feedback regarding ranomizing Guid bits --- .../src/System/Diagnostics/RandomNumberGenerator.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs index a080c930b5b3c..89830687429b7 100755 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs @@ -39,6 +39,11 @@ public unsafe RandomNumberGenerator() _s1 = *(g1p + 1); _s2 = *g2p; _s3 = *(g2p + 1); + + // Guid uses the 4 most significant bits of the first long as the version which would be fixed and not randomized. + // Let's randomize these bits using the 4 most significant bits from the second long. + _s0 = (_s0 & 0x0FFFFFFFFFFFFFFF) | (_s1 & 0xF000000000000000); + _s2 = (_s2 & 0x0FFFFFFFFFFFFFFF) | (_s3 & 0xF000000000000000); } while ((_s0 | _s1 | _s2 | _s3) == 0); } From a0d9f566fdcc06add0eabbdb19de95a760a10cba Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 20 Jan 2021 17:56:32 -0800 Subject: [PATCH 7/8] address more Feedback --- .../src/System/Diagnostics/RandomNumberGenerator.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs index 89830687429b7..8d9d5600aba3b 100755 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs @@ -41,9 +41,12 @@ public unsafe RandomNumberGenerator() _s3 = *(g2p + 1); // Guid uses the 4 most significant bits of the first long as the version which would be fixed and not randomized. - // Let's randomize these bits using the 4 most significant bits from the second long. + // and uses 2 other bits in the second long for variants which would be fixed and not randomized too. + // let's overwrite the fixed bits in each long part by the other long. _s0 = (_s0 & 0x0FFFFFFFFFFFFFFF) | (_s1 & 0xF000000000000000); _s2 = (_s2 & 0x0FFFFFFFFFFFFFFF) | (_s3 & 0xF000000000000000); + _s1 = (_s1 & 0xFFFFFFFFFFFFFF3F) | (_s0 & 0x00000000000000C0); + _s3 = (_s3 & 0xFFFFFFFFFFFFFF3F) | (_s2 & 0x00000000000000C0); } while ((_s0 | _s1 | _s2 | _s3) == 0); } From 9416da294cb7ccafc4bb045aada780b5631cee20 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 21 Jan 2021 09:54:51 -0800 Subject: [PATCH 8/8] Address more feedback --- .../src/System/Diagnostics/Activity.cs | 2 +- .../src/System/Diagnostics/RandomNumberGenerator.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index 1dc871e8fdaff..346c5e56796c3 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -1667,7 +1667,7 @@ internal static unsafe void SetToRandomBytes(Span outBytes) Unsafe.WriteUnaligned(ref outBytes[0], r.Next()); - if (outBytes.Length >= 16) + if (outBytes.Length == 16) { Unsafe.WriteUnaligned(ref outBytes[8], r.Next()); } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs index 8d9d5600aba3b..dd151339b5cf4 100755 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/RandomNumberGenerator.cs @@ -4,7 +4,7 @@ namespace System.Diagnostics { /// - /// RandomNumberGenerator implementation is the 64-bit random number generator based on the Xorshift algorithm (known as shift-register generators). + /// RandomNumberGenerator implementation is the 64-bit random number generator based on the Xoshiro256StarStar algorithm (known as shift-register generators). /// internal class RandomNumberGenerator {