From bccb2f43c9b19684cff4f0a37a49db19acb33df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Lema=C5=84czyk?= Date: Mon, 13 Oct 2025 19:56:47 +0200 Subject: [PATCH 1/4] Document unreachable guards in GPU by-divisor pipeline --- ...MersenneNumberDivisorByDivisorCpuTester.cs | 97 ++++++++++-------- ...MersenneNumberDivisorByDivisorGpuTester.cs | 98 +++++++++++-------- ...senneNumberDivisorCandidateGpuEvaluator.cs | 26 +++-- ...rsenneNumberDivisorMontgomeryGpuBuilder.cs | 17 +++- ...ersenneNumberDivisorRemainderGpuStepper.cs | 31 +++--- ...ersenneNumberDivisorResidueGpuEvaluator.cs | 19 ++-- .../MersenneNumberDivisorByDivisorTester.cs | 12 ++- 7 files changed, 178 insertions(+), 122 deletions(-) diff --git a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs index fed7e76..b74761f 100644 --- a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs +++ b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs @@ -146,10 +146,11 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv public void PrepareCandidates(ReadOnlySpan primes, Span allowedMaxValues) { - if (allowedMaxValues.Length < primes.Length) - { - throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); - } + // EvenPerfectBitScanner always provides equally sized spans here, so the historical guard never triggers. + // if (allowedMaxValues.Length < primes.Length) + // { + // throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); + // } ulong divisorLimit; @@ -236,11 +237,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv } UInt128 step = (UInt128)prime << 1; - if (step == UInt128.Zero) - { - processedAll = true; - return false; - } + // EvenPerfectBitScanner only supplies primes greater than one here, so the shifted step never becomes zero. + // if (step == UInt128.Zero) + // { + // processedAll = true; + // return false; + // } UInt128 limit = allowedMax; UInt128 divisor = step + UInt128.One; @@ -321,10 +323,11 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv return true; } - if (divisorCycle == 0UL) - { - Console.WriteLine($"Divisor cycle was not calculated for {prime}"); - } + // DivisorCycleCache guarantees a non-zero cycle for every divisor reached on the EvenPerfectBitScanner path. + // if (divisorCycle == 0UL) + // { + // Console.WriteLine($"Divisor cycle was not calculated for {prime}"); + // } } divisor += step; @@ -369,11 +372,13 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv GpuUInt128 step128 = new GpuUInt128(prime); step128.ShiftLeft(1); - if (step128.IsZero) - { - processedAll = true; - return false; - } + // Primes above one yield a non-zero 2p value even after widening to GpuUInt128, so the defensive + // zero guard remains commented out to document the invariant without branching. + // if (step128.IsZero) + // { + // processedAll = true; + // return false; + // } GpuUInt128 limit128 = new GpuUInt128(allowedMax); GpuUInt128 divisor128 = step128; @@ -449,10 +454,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv int chunkCount = remainingCandidates > (ulong)batchCapacity ? batchCapacity : (int)remainingCandidates; - if (chunkCount <= 0) - { - break; - } + // batchCapacity and remainingCandidates are strictly positive in this path, so the zero-count + // shortcut never executes on production scans. + // if (chunkCount <= 0) + // { + // break; + // } Span candidateSpan = candidateBuffer.AsSpan(0, chunkCount); Span maskSpan = maskBuffer.AsSpan(0, chunkCount); @@ -604,11 +611,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv } ulong interval = PerfectNumberConstants.ConsoleInterval; - if (interval == 0UL) - { - _lastStatusDivisor = 0UL; - return; - } + // PerfectNumberConstants.ConsoleInterval is a positive constant, so this guard never triggers on EvenPerfectBitScanner. + // if (interval == 0UL) + // { + // _lastStatusDivisor = 0UL; + // return; + // } ulong total = _lastStatusDivisor + processedCount; // TODO: Replace this modulo with the ring-buffer style counter (subtract loop) used in the fast CLI @@ -696,11 +704,14 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv } ulong modulus = effectiveDivisorData.Modulus; - if (modulus <= 1UL || (modulus & 1UL) == 0UL) - { - hits.Clear(); - return; - } + // EvenPerfectBitScanner only schedules odd divisors greater than one, and the fallback + // above rebuilds Montgomery data when the modulus mismatches, so this guard never + // triggers for production scans. + // if (modulus <= 1UL || (modulus & 1UL) == 0UL) + // { + // hits.Clear(); + // return; + // } if (divisorCycle == 0UL) { @@ -803,10 +814,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv private void EnsureCapacity(int requiredLength) { - if (requiredLength <= 0) - { - return; - } + // Prime batches are never empty on the by-divisor scanner path, so the zero-length guard stays commented + // out to remove the redundant branch. + // if (requiredLength <= 0) + // { + // return; + // } // Sessions release pooled buffers between scans; a single null check covers every array because buffers are rented and returned together. if (requiredLength <= _bufferCapacity && _primeDeltas is not null) @@ -873,11 +886,13 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv private static void ComputeCycleRemainders(ulong cycleLength, ReadOnlySpan primes, ReadOnlySpan deltas, Span remainders) { - if (cycleLength == 0UL || primes.Length == 0) - { - remainders.Clear(); - return; - } + // Divisor cycles are validated before reaching the GPU helper, and prime batches are non-empty, so + // the defensive zero checks remain commented out here. + // if (cycleLength == 0UL || primes.Length == 0) + // { + // remainders.Clear(); + // return; + // } ulong remainder = primes[0] % cycleLength; remainders[0] = remainder; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs index 9d10387..2974517 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs @@ -87,11 +87,13 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv batchCapacity = _gpuBatchSize; } - if (allowedMax < 3UL) - { - divisorsExhausted = true; - return true; - } + // The GPU path always receives an allowed maximum well above the minimal divisor, so the legacy + // guard stays commented out to document the invariant without branching. + // if (allowedMax < 3UL) + // { + // divisorsExhausted = true; + // return true; + // } bool composite; bool coveredRange; @@ -222,7 +224,8 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv ArrayView1D exponentView; ArrayView1D hitsView; - cycleCapacity = Math.Max(1, cycleCapacity); + // Batch resources expose a positive cycle capacity, so skip the redundant clamp here. + // cycleCapacity = Math.Max(1, cycleCapacity); int chunkCapacity = Math.Min(batchCapacity, cycleCapacity); int maxThreadsPerGroup = (int)accelerator.MaxNumThreadsPerGroup; @@ -231,20 +234,22 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv chunkCapacity = Math.Min(chunkCapacity, maxThreadsPerGroup); } - if (chunkCapacity <= 0) - { - chunkCapacity = cycleCapacity; - } + // cycleCapacity already bounds chunkCapacity above zero, so the fallback would never execute. + // if (chunkCapacity <= 0) + // { + // chunkCapacity = cycleCapacity; + // } ulong[] filteredDivisors = ArrayPool.Shared.Rent(chunkCapacity); ulong[] divisorGaps = ArrayPool.Shared.Rent(chunkCapacity); UInt128 twoP128 = (UInt128)prime << 1; - if (twoP128 == UInt128.Zero) - { - coveredRange = true; - return false; - } + // Primes above one yield a non-zero 2p value, so the legacy zero guard remains commented out. + // if (twoP128 == UInt128.Zero) + // { + // coveredRange = true; + // return false; + // } UInt128 allowedMax128 = allowedMax; UInt128 firstDivisor128 = twoP128 + UInt128.One; @@ -255,11 +260,13 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv } UInt128 maxK128 = (allowedMax128 - UInt128.One) / twoP128; - if (maxK128 == UInt128.Zero) - { - coveredRange = true; - return false; - } + // Once the first divisor falls within the allowed range, at least one multiplier k is admissible, + // so keep the defensive zero check commented out. + // if (maxK128 == UInt128.Zero) + // { + // coveredRange = true; + // return false; + // } ulong maxK = maxK128 > ulong.MaxValue ? ulong.MaxValue : (ulong)maxK128; ulong currentK = 1UL; @@ -301,10 +308,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv chunkCount = (int)remainingK; } - if (chunkCount <= 0) - { - break; - } + // chunkCapacity and the remaining multiplier range are strictly positive here, so the zero + // chunk fallback never triggers on production scans. + // if (chunkCount <= 0) + // { + // break; + // } Span candidateSpan = cycleCandidates.AsSpan(0, chunkCount); Span gapSpan = divisorGaps.AsSpan(0, chunkCount); @@ -465,10 +474,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv Action, ArrayView, byte> deltaKernel, Action, ArrayView, byte, byte> scanKernel) { - if (count <= 0) - { - return; - } + // GPU batches always contain at least one divisor gap, so the empty-count guard remains commented + // to eliminate redundant branching. + // if (count <= 0) + // { + // return; + // } deltaKernel(count, gapView, deltaView, modulus); SharedMemoryConfig sharedMemoryConfig = SharedMemoryConfig.RequestDynamic(count); @@ -491,11 +502,13 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv Span destination, Action, ArrayView, ArrayView, ArrayView, byte, ArrayView> maskKernel) { - if (count <= 0) - { - destination.Clear(); - return; - } + // Candidate batches always provide at least one entry, so the zero-count guard stays commented out + // to avoid clearing spans unnecessarily. + // if (count <= 0) + // { + // destination.Clear(); + // return; + // } maskKernel(count, remainder10View, remainder8View, remainder5View, remainder3View, lastIsSevenFlag, maskView); maskView.CopyToCPU(ref MemoryMarshal.GetReference(destination), count); @@ -503,10 +516,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv private static byte FetchLastRemainder(ArrayView1D remainderView, int count) { - if (count <= 0) - { - return 0; - } + // Kernels always process at least one remainder in this path, so the empty-span guard remains + // commented to document the invariant. + // if (count <= 0) + // { + // return 0; + // } byte value = 0; remainderView.SubView(count - 1, 1).CopyToCPU(ref value, 1); @@ -519,10 +534,11 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv if (result >= modulus) { result -= modulus; - if (result >= modulus) - { - result %= modulus; - } + // The step deltas remain below the modulus, so a single subtraction suffices on this path. + // if (result >= modulus) + // { + // result %= modulus; + // } } return (byte)result; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs index e4e0fe1..b64aad2 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs @@ -68,10 +68,12 @@ internal sealed class MersenneNumberDivisorCandidateGpuEvaluator : IDisposable } int count = candidates.Length; - if (count == 0) - { - return; - } + // Candidate batches always carry at least one divisor on the by-divisor path, so the empty-count + // guard remains commented out to eliminate the branch. + // if (count == 0) + // { + // return; + // } EnsureCapacity(count); @@ -115,7 +117,12 @@ internal sealed class MersenneNumberDivisorCandidateGpuEvaluator : IDisposable return; } - int newCapacity = _capacity == 0 ? 1 : _capacity; + int newCapacity = _capacity; + // The evaluator always starts with a positive capacity, so the zero-capacity fallback remains commented. + // if (newCapacity == 0) + // { + // newCapacity = 1; + // } while (newCapacity < required) { newCapacity <<= 1; @@ -188,10 +195,11 @@ internal sealed class MersenneNumberDivisorCandidateGpuEvaluator : IDisposable private static int ComputeRemainder(byte remainder, byte step, int index, int modulus) { - if (modulus == 0) - { - return 0; - } + // Modulus lookups only cover {10, 8, 5, 3, 7, 11}, so the zero-modulus guard stays commented out. + // if (modulus == 0) + // { + // return 0; + // } long value = remainder; value += (long)step * index; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs index 5c290a2..6b8a163 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs @@ -54,10 +54,12 @@ internal sealed class MersenneNumberDivisorMontgomeryGpuBuilder : IDisposable } int length = moduli.Length; - if (length == 0) - { - return; - } + // Montgomery batches always include at least one modulus on the by-divisor path, so the empty-length + // guard stays commented out to avoid branching. + // if (length == 0) + // { + // return; + // } EnsureCapacity(length); @@ -83,7 +85,12 @@ internal sealed class MersenneNumberDivisorMontgomeryGpuBuilder : IDisposable return; } - int newCapacity = _capacity == 0 ? 1 : _capacity; + int newCapacity = _capacity; + // The builder always starts with a positive capacity, so the zero-capacity fallback remains commented. + // if (newCapacity == 0) + // { + // newCapacity = 1; + // } while (newCapacity < required) { newCapacity <<= 1; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs index e293430..9dd132f 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs @@ -71,10 +71,12 @@ internal sealed class MersenneNumberDivisorRemainderGpuStepper : IDisposable throw new ObjectDisposedException(nameof(MersenneNumberDivisorRemainderGpuStepper)); } - if (advanceCount <= 0 || remainders.Length == 0) - { - return; - } + // GPU remainder batches always advance by a positive chunk count and operate on fixed-size tables, so + // the defensive early-return remains commented out. + // if (advanceCount <= 0 || remainders.Length == 0) + // { + // return; + // } if (remainders.Length != _length) { @@ -99,17 +101,20 @@ internal sealed class MersenneNumberDivisorRemainderGpuStepper : IDisposable private static void AdvanceRemaindersKernel(Index1D index, int advanceCount, ArrayView remainders, ArrayView steps, ArrayView results) { int idx = index; - if (idx >= remainders.Length) - { - return; - } + // Kernel launches match the remainder table length, so the bounds guard stays commented to avoid redundant branching. + // if (idx >= remainders.Length) + // { + // return; + // } int modulus = GetModulus(idx); - if (modulus == 0) - { - results[idx] = remainders[idx]; - return; - } + // The modulus selector only emits positive bases (10, 8, 5, 3, 7, 11) for the configured indices, so the zero-modulus + // fallback remains commented out. + // if (modulus == 0) + // { + // results[idx] = remainders[idx]; + // return; + // } int remainder = remainders[idx] % modulus; int step = steps[idx] % modulus; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs index f6c7d9f..8970fc2 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs @@ -51,10 +51,12 @@ internal sealed class MersenneNumberDivisorResidueGpuEvaluator : IDisposable } int length = exponents.Length; - if (length == 0) - { - return; - } + // Residue batches always include at least one exponent when scheduled from the by-divisor scanner, so + // the empty-length guard remains commented out to avoid branching. + // if (length == 0) + // { + // return; + // } EnsureCapacity(length); @@ -79,10 +81,11 @@ internal sealed class MersenneNumberDivisorResidueGpuEvaluator : IDisposable } int newCapacity = _capacity; - if (newCapacity == 0) - { - newCapacity = 1; - } + // The evaluator always starts with a positive capacity, so the zero-capacity fallback stays commented. + // if (newCapacity == 0) + // { + // newCapacity = 1; + // } while (newCapacity < required) { diff --git a/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs b/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs index 2511190..508c06d 100644 --- a/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs +++ b/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs @@ -180,11 +180,13 @@ public static class MersenneNumberDivisorByDivisorTester return; } - if (maxPrime <= 1UL) - { - Console.WriteLine("The filter specified by --filter-p must contain at least one prime exponent greater than 1 for --mersenne=bydivisor."); - return; - } + // EvenPerfectBitScanner removes candidates <= 1 before reaching this point, + // so this guard never triggers during production runs. + // if (maxPrime <= 1UL) + // { + // Console.WriteLine("The filter specified by --filter-p must contain at least one prime exponent greater than 1 for --mersenne=bydivisor."); + // return; + // } tester.ConfigureFromMaxPrime(maxPrime); From 6e89700f80c75d98110ee6828f76e47ef1317114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Lema=C5=84czyk?= Date: Mon, 13 Oct 2025 20:41:00 +0200 Subject: [PATCH 2/4] Clarify invariants across GPU divisor helpers --- ...MersenneNumberDivisorByDivisorCpuTester.cs | 50 ++++++++----------- ...MersenneNumberDivisorByDivisorGpuTester.cs | 28 ++++------- ...senneNumberDivisorCandidateGpuEvaluator.cs | 5 +- ...ersenneNumberDivisorRemainderGpuStepper.cs | 11 ++-- ...ersenneNumberDivisorResidueGpuEvaluator.cs | 2 +- 5 files changed, 44 insertions(+), 52 deletions(-) diff --git a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs index b74761f..1e9e937 100644 --- a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs +++ b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs @@ -152,18 +152,14 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv // throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); // } - ulong divisorLimit; - - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - divisorLimit = _divisorLimit; + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + // Configuration is immutable after startup, so reading the cached limit does not require locking. + ulong divisorLimit = _divisorLimit; + for (int index = 0; index < primes.Length; index++) { allowedMaxValues[index] = ComputeAllowedMaxDivisor(primes[index], divisorLimit); @@ -172,21 +168,18 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv public IMersenneNumberDivisorByDivisorTester.IDivisorScanSession CreateDivisorSession() { - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - if (_sessionPool.TryTake(out DivisorScanSession? session)) - { - session.Reset(); - return session; - } - - return new DivisorScanSession(this); + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + + if (_sessionPool.TryTake(out DivisorScanSession? session)) + { + session.Reset(); + return session; + } + + return new DivisorScanSession(this); } private void ReturnSession(DivisorScanSession session) @@ -323,11 +316,11 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv return true; } - // DivisorCycleCache guarantees a non-zero cycle for every divisor reached on the EvenPerfectBitScanner path. - // if (divisorCycle == 0UL) - // { - // Console.WriteLine($"Divisor cycle was not calculated for {prime}"); - // } + if (divisorCycle == 0UL) + { + // Keep this guard active to surface mismatches between generated candidates and the snapshot cache. + Console.WriteLine($"Divisor cycle was not calculated for {prime}"); + } } divisor += step; @@ -830,7 +823,7 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv int newCapacity = _bufferCapacity; if (newCapacity == 0) { - // Fresh sessions start without buffers; seed the first rent with a single-slot array. + // EvenPerfectBitScanner hits this branch on the first batch of each session, so keep the seed allocation. newCapacity = 1; } @@ -908,6 +901,7 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv { if (modulus == 0UL) { + // A zero modulus indicates a missing cycle entry; preserve the fallback so snapshot regressions remain visible. return 0UL; } diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs index 2974517..5fc08b4 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs @@ -64,28 +64,22 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv public void ConfigureFromMaxPrime(ulong maxPrime) { - lock (_sync) - { - _divisorLimit = ComputeDivisorLimitFromMaxPrime(maxPrime); - _isConfigured = true; - } + // Configuration happens once during startup, so no synchronization is required to cache the divisor limit. + _divisorLimit = ComputeDivisorLimitFromMaxPrime(maxPrime); + _isConfigured = true; } public bool IsPrime(ulong prime, out bool divisorsExhausted, TimeSpan? timeLimit = null) { - ulong allowedMax; - int batchCapacity; + // The CLI always configures the tester during startup, so the historical guard is commented out to avoid branching. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } - lock (_sync) - { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - allowedMax = ComputeAllowedMaxDivisor(prime, _divisorLimit); - batchCapacity = _gpuBatchSize; - } + // Configuration is immutable, so the cached values can be read without locking. + ulong allowedMax = ComputeAllowedMaxDivisor(prime, _divisorLimit); + int batchCapacity = _gpuBatchSize; // The GPU path always receives an allowed maximum well above the minimal divisor, so the legacy // guard stays commented out to document the invariant without branching. diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs index b64aad2..4177fc7 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs @@ -75,7 +75,10 @@ internal sealed class MersenneNumberDivisorCandidateGpuEvaluator : IDisposable // return; // } - EnsureCapacity(count); + if (count > _capacity) + { + EnsureCapacity(count); + } Span hostCandidateSpan = _hostCandidates.AsSpan(0, count); Span hostMaskSpan = _hostMask.AsSpan(0, count); diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs index 9dd132f..721a519 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs @@ -52,7 +52,7 @@ internal sealed class MersenneNumberDivisorRemainderGpuStepper : IDisposable { if (_disposed) { - return; + throw new InvalidOperationException("MersenneNumberDivisorRemainderGpuStepper was disposed more than once."); } _disposed = true; @@ -78,10 +78,11 @@ internal sealed class MersenneNumberDivisorRemainderGpuStepper : IDisposable // return; // } - if (remainders.Length != _length) - { - throw new ArgumentException("Remainder buffer length must match the configured length."); - } + // Production code always calls the stepper with a fixed-length table, so the historical length guard stays commented. + // if (remainders.Length != _length) + // { + // throw new ArgumentException("Remainder buffer length must match the configured length."); + // } Span hostRemainderSpan = _hostRemainders.AsSpan(0, _length); remainders.CopyTo(hostRemainderSpan); diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs index 8970fc2..72c60ad 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs @@ -33,7 +33,7 @@ internal sealed class MersenneNumberDivisorResidueGpuEvaluator : IDisposable { if (_disposed) { - return; + throw new InvalidOperationException("MersenneNumberDivisorResidueGpuEvaluator was disposed more than once."); } _disposed = true; From b0ba6401cae81857f2c81690d017a85f59eb1643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Lema=C5=84czyk?= Date: Mon, 13 Oct 2025 20:41:07 +0200 Subject: [PATCH 3/4] Remove configuration locks from by-divisor testers --- ...MersenneNumberDivisorByDivisorCpuTester.cs | 102 ++++++++---------- ...MersenneNumberDivisorByDivisorGpuTester.cs | 47 +++----- 2 files changed, 61 insertions(+), 88 deletions(-) diff --git a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs index 1e9e937..7e9d05d 100644 --- a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs +++ b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs @@ -3,6 +3,7 @@ using System.Buffers; using System.Collections.Concurrent; using System.Collections.Generic; using System.Numerics; +using System.Threading; using PerfectNumbers.Core; using PerfectNumbers.Core.Gpu; @@ -10,14 +11,11 @@ namespace PerfectNumbers.Core.Cpu; public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDivisorByDivisorTester { - private readonly object _sync = new(); private readonly ConcurrentBag _sessionPool = new(); private ulong _divisorLimit; - private ulong _lastStatusDivisor; + private int _lastStatusDivisor; private bool _isConfigured; private int _batchSize = 1_024; - - private ulong _configuredMaxPrime; private ByDivisorDeltasDevice _deltasDevice = ByDivisorDeltasDevice.Cpu; private ByDivisorMontgomeryDevice _montgomeryDevice = ByDivisorMontgomeryDevice.Cpu; @@ -34,80 +32,55 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv public ByDivisorDeltasDevice DeltasDevice { get => _deltasDevice; - set - { - lock (_sync) - { - _deltasDevice = value; - } - } + set => _deltasDevice = value; } public ByDivisorMontgomeryDevice MontgomeryDevice { get => _montgomeryDevice; - set - { - lock (_sync) - { - _montgomeryDevice = value; - } - } + set => _montgomeryDevice = value; } public void ConfigureFromMaxPrime(ulong maxPrime) { - lock (_sync) - { - _configuredMaxPrime = maxPrime; - _divisorLimit = ComputeDivisorLimitFromMaxPrime(maxPrime); - _lastStatusDivisor = 0UL; - _isConfigured = true; - } + _divisorLimit = ComputeDivisorLimitFromMaxPrime(maxPrime); + _lastStatusDivisor = 0; + _isConfigured = true; } public ulong DivisorLimit { get { - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - return _divisorLimit; + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + + return _divisorLimit; } } public ulong GetAllowedMaxDivisor(ulong prime) { - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - return ComputeAllowedMaxDivisor(prime, _divisorLimit); + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + + return ComputeAllowedMaxDivisor(prime, _divisorLimit); } public bool IsPrime(ulong prime, out bool divisorsExhausted, TimeSpan? timeLimit = null) { - ulong allowedMax; - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - allowedMax = ComputeAllowedMaxDivisor(prime, _divisorLimit); + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + ulong divisorLimit = _divisorLimit; + ulong allowedMax = ComputeAllowedMaxDivisor(prime, divisorLimit); + if (allowedMax < 3UL) { divisorsExhausted = true; @@ -128,10 +101,7 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv if (processedCount > 0UL) { - lock (_sync) - { - UpdateStatusUnsafe(processedCount); - } + UpdateStatusUnsafe(processedCount); } if (composite) @@ -603,18 +573,34 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv return; } - ulong interval = PerfectNumberConstants.ConsoleInterval; + int interval = PerfectNumberConstants.ConsoleInterval; // PerfectNumberConstants.ConsoleInterval is a positive constant, so this guard never triggers on EvenPerfectBitScanner. - // if (interval == 0UL) + // if (interval <= 0) // { - // _lastStatusDivisor = 0UL; + // Volatile.Write(ref _lastStatusDivisor, 0); // return; // } - ulong total = _lastStatusDivisor + processedCount; - // TODO: Replace this modulo with the ring-buffer style counter (subtract loop) used in the fast CLI - // status benchmarks so we avoid `%` in this hot loop while still wrapping progress correctly. - _lastStatusDivisor = total % interval; + int processedDelta = (int)(processedCount % (uint)interval); + if (processedDelta == 0) + { + return; + } + + while (true) + { + int previous = Volatile.Read(ref _lastStatusDivisor); + int total = previous + processedDelta; + if (total >= interval) + { + total -= interval; + } + + if (previous == Interlocked.CompareExchange(ref _lastStatusDivisor, total, previous)) + { + return; + } + } } private static ulong ComputeDivisorLimitFromMaxPrime(ulong maxPrime) diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs index 5fc08b4..b3aa033 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs @@ -19,7 +19,6 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv private const int RemainderSlotCount = 4; private int _gpuBatchSize = GpuConstants.ScanBatchSize; - private readonly object _sync = new(); private ulong _divisorLimit; private bool _isConfigured; @@ -151,18 +150,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); } - ulong divisorLimit; - - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - divisorLimit = _divisorLimit; + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + ulong divisorLimit = _divisorLimit; for (int index = 0; index < primes.Length; index++) { allowedMaxValues[index] = ComputeAllowedMaxDivisor(primes[index], divisorLimit); @@ -943,34 +936,28 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv public ulong GetAllowedMaxDivisor(ulong prime) { - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - return ComputeAllowedMaxDivisor(prime, _divisorLimit); + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + + return ComputeAllowedMaxDivisor(prime, _divisorLimit); } public DivisorScanSession CreateDivisorSession() { - lock (_sync) + if (!_isConfigured) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } - - if (_sessionPool.TryTake(out DivisorScanSession? session)) - { - session.Reset(); - return session; - } - - return new DivisorScanSession(this); + throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); } + + if (_sessionPool.TryTake(out DivisorScanSession? session)) + { + session.Reset(); + return session; + } + + return new DivisorScanSession(this); } IMersenneNumberDivisorByDivisorTester.IDivisorScanSession IMersenneNumberDivisorByDivisorTester.CreateDivisorSession() => CreateDivisorSession(); From f2eb6580c42a793b553b0c50b36441998ddd1544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Lema=C5=84czyk?= Date: Mon, 13 Oct 2025 20:41:14 +0200 Subject: [PATCH 4/4] Comment unreachable configuration guards in by-divisor testers --- ...MersenneNumberDivisorByDivisorCpuTester.cs | 50 +++++++++++-------- ...MersenneNumberDivisorByDivisorGpuTester.cs | 30 ++++++----- 2 files changed, 48 insertions(+), 32 deletions(-) diff --git a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs index 7e9d05d..5e1d549 100644 --- a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs +++ b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs @@ -52,10 +52,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv { get { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // The by-divisor driver configures the tester during startup, so exposing the divisor limit never + // re-enters this method before initialization completes. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } return _divisorLimit; } @@ -63,20 +65,24 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv public ulong GetAllowedMaxDivisor(ulong prime) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // The CLI wires the tester through ConfigureFromMaxPrime before invoking query helpers, so the guard + // would never trigger in production runs. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } return ComputeAllowedMaxDivisor(prime, _divisorLimit); } public bool IsPrime(ulong prime, out bool divisorsExhausted, TimeSpan? timeLimit = null) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // The tester is configured once during startup, so runtime IsPrime calls always observe a ready + // configuration and the legacy guard can remain commented out. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } ulong divisorLimit = _divisorLimit; ulong allowedMax = ComputeAllowedMaxDivisor(prime, divisorLimit); @@ -122,10 +128,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv // throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); // } - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // PrepareCandidates is scheduled only after ConfigureFromMaxPrime succeeds, so the tester state is + // stable here and the defensive guard is no longer needed. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } // Configuration is immutable after startup, so reading the cached limit does not require locking. ulong divisorLimit = _divisorLimit; @@ -138,10 +146,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv public IMersenneNumberDivisorByDivisorTester.IDivisorScanSession CreateDivisorSession() { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // The session pool is populated only after configuration, so consumers never request a session before + // ConfigureFromMaxPrime completes. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } if (_sessionPool.TryTake(out DivisorScanSession? session)) { diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs index b3aa033..f0f7e65 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs @@ -150,10 +150,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); } - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // The GPU pipeline calls ConfigureFromMaxPrime before preparing candidates, so the legacy guard + // remains commented out to document the invariant without branching. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } ulong divisorLimit = _divisorLimit; for (int index = 0; index < primes.Length; index++) @@ -936,20 +938,24 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv public ulong GetAllowedMaxDivisor(ulong prime) { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // The CLI configures the GPU tester once at startup, so callers never request divisor limits before + // initialization completes. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } return ComputeAllowedMaxDivisor(prime, _divisorLimit); } public DivisorScanSession CreateDivisorSession() { - if (!_isConfigured) - { - throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); - } + // Divisor sessions are handed out only after configuration, so this guard would never trigger on the + // production path. + // if (!_isConfigured) + // { + // throw new InvalidOperationException("ConfigureFromMaxPrime must be called before using the tester."); + // } if (_sessionPool.TryTake(out DivisorScanSession? session)) {