Skip to content

Commit 9b4af52

Browse files
committed
Harden atomic double addition
1 parent 313d7ba commit 9b4af52

2 files changed

Lines changed: 154 additions & 5 deletions

File tree

‎Numerics/Utilities/Tools.cs‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Collections.Generic;
44
using System.IO;
55
using System.IO.Compression;
6+
using System.Runtime.CompilerServices;
67
using System.Threading;
78

89
namespace Numerics
@@ -666,20 +667,55 @@ public static double Max(IList<double> values, IList<int> indicators, bool useCo
666667
/// </summary>
667668
/// <param name="valueToAddTo">The accumulator to update atomically.</param>
668669
/// <param name="valueToAdd">The value to add.</param>
669-
/// <returns>The value committed by the successful compare-and-swap operation.</returns>
670+
/// <returns>The accumulator value produced by the atomic addition.</returns>
670671
/// <remarks>
671-
/// Concurrent updates are not lost, but their order depends on thread scheduling. Floating-point
672-
/// reductions using this method are therefore not guaranteed to be bit-reproducible.
672+
/// All concurrent writes to <paramref name="valueToAddTo"/> must use this method or another
673+
/// interlocked operation. The compare-and-swap operation can retry under contention. Concurrent
674+
/// updates are not lost, but their order depends on thread scheduling, so floating-point reductions
675+
/// using this method are not guaranteed to be bit-reproducible.
673676
/// </remarks>
677+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
674678
public static double ParallelAdd(ref double valueToAddTo, double valueToAdd)
675679
{
676-
double newCurrentValue = valueToAddTo;
680+
double currentValue = valueToAddTo;
681+
double newValue = currentValue + valueToAdd;
682+
double observedValue = Interlocked.CompareExchange(ref valueToAddTo, newValue, currentValue);
683+
if (observedValue == currentValue &&
684+
(currentValue != 0d ||
685+
BitConverter.DoubleToInt64Bits(observedValue) == BitConverter.DoubleToInt64Bits(currentValue)))
686+
{
687+
return newValue;
688+
}
689+
if (double.IsNaN(observedValue) && double.IsNaN(currentValue)) return newValue;
690+
return ParallelAddRetry(ref valueToAddTo, valueToAdd, observedValue);
691+
}
692+
693+
/// <summary>
694+
/// Retries an atomic double addition after a failed compare-and-swap operation.
695+
/// </summary>
696+
/// <param name="valueToAddTo">The accumulator to update atomically.</param>
697+
/// <param name="valueToAdd">The value to add.</param>
698+
/// <param name="observedValue">The accumulator value observed by the failed operation.</param>
699+
/// <returns>The accumulator value produced by the atomic addition.</returns>
700+
/// <remarks>
701+
/// The observed value seeds the lock-free retry loop so that another non-volatile read is unnecessary.
702+
/// Signed zeros require a representation comparison because numerical equality does not distinguish
703+
/// positive zero from negative zero, while compare-and-swap does.
704+
/// </remarks>
705+
private static double ParallelAddRetry(ref double valueToAddTo, double valueToAdd, double observedValue)
706+
{
707+
double newCurrentValue = observedValue;
677708
while (true)
678709
{
679710
double currentValue = newCurrentValue;
680711
double newValue = currentValue + valueToAdd;
681712
newCurrentValue = Interlocked.CompareExchange(ref valueToAddTo, newValue, currentValue);
682-
if (newCurrentValue == currentValue) return newValue;
713+
if (newCurrentValue == currentValue &&
714+
(currentValue != 0d ||
715+
BitConverter.DoubleToInt64Bits(newCurrentValue) == BitConverter.DoubleToInt64Bits(currentValue)))
716+
{
717+
return newValue;
718+
}
683719
if (double.IsNaN(newCurrentValue) && double.IsNaN(currentValue)) return newValue;
684720
}
685721
}

‎Test_Numerics/Utilities/Test_Tools.cs‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Numerics;
33
using System;
44
using System.Collections.Generic;
5+
using System.Threading;
56
using System.Threading.Tasks;
67

78
namespace Utilities
@@ -374,6 +375,118 @@ public void Test_ParallelAdd_NaNAccumulatorTerminates()
374375
Assert.IsTrue(double.IsNaN(result));
375376
}
376377

378+
/// <summary>
379+
/// Verifies signed zero, NaN, and infinity follow double-precision addition semantics.
380+
/// </summary>
381+
[TestMethod]
382+
public void Test_ParallelAdd_HandlesSpecialValues()
383+
{
384+
double negativeZero = BitConverter.Int64BitsToDouble(long.MinValue);
385+
386+
double total = negativeZero;
387+
double result = Tools.ParallelAdd(ref total, negativeZero);
388+
389+
Assert.AreEqual(long.MinValue, BitConverter.DoubleToInt64Bits(total));
390+
Assert.AreEqual(long.MinValue, BitConverter.DoubleToInt64Bits(result));
391+
392+
total = negativeZero;
393+
result = Tools.ParallelAdd(ref total, 0d);
394+
395+
Assert.AreEqual(0L, BitConverter.DoubleToInt64Bits(total));
396+
Assert.AreEqual(0L, BitConverter.DoubleToInt64Bits(result));
397+
398+
total = 1d;
399+
result = Tools.ParallelAdd(ref total, double.NaN);
400+
401+
Assert.IsTrue(double.IsNaN(total));
402+
Assert.IsTrue(double.IsNaN(result));
403+
404+
total = double.PositiveInfinity;
405+
result = Tools.ParallelAdd(ref total, 1d);
406+
407+
Assert.AreEqual(double.PositiveInfinity, total);
408+
Assert.AreEqual(double.PositiveInfinity, result);
409+
410+
total = double.NegativeInfinity;
411+
result = Tools.ParallelAdd(ref total, -1d);
412+
413+
Assert.AreEqual(double.NegativeInfinity, total);
414+
Assert.AreEqual(double.NegativeInfinity, result);
415+
416+
total = double.PositiveInfinity;
417+
result = Tools.ParallelAdd(ref total, double.NegativeInfinity);
418+
419+
Assert.IsTrue(double.IsNaN(total));
420+
Assert.IsTrue(double.IsNaN(result));
421+
}
422+
423+
/// <summary>
424+
/// Verifies concurrent mixed-sign additions are all committed when the accumulator revisits values.
425+
/// </summary>
426+
[TestMethod]
427+
public void Test_ParallelAdd_CommitsMixedSignUpdates()
428+
{
429+
const int updateCount = 25000;
430+
double total = 0d;
431+
432+
Parallel.For(
433+
0,
434+
updateCount,
435+
index => Tools.ParallelAdd(ref total, index % 2 == 0 ? 2d : -1d));
436+
437+
Assert.AreEqual(updateCount / 2d, total);
438+
}
439+
440+
/// <summary>
441+
/// Verifies a negative-to-positive zero race cannot cause finite concurrent updates to be lost.
442+
/// </summary>
443+
[TestMethod]
444+
public void Test_ParallelAdd_SignedZeroContentionDoesNotLoseUpdates()
445+
{
446+
const int rounds = 256;
447+
double total = 0d;
448+
bool everyUpdateCommitted = true;
449+
double negativeZero = BitConverter.Int64BitsToDouble(long.MinValue);
450+
451+
using (var startBarrier = new Barrier(5))
452+
using (var finishBarrier = new Barrier(5))
453+
{
454+
Func<double, Task> createWorker = valueToAdd => Task.Factory.StartNew(
455+
() =>
456+
{
457+
for (int round = 0; round < rounds; round++)
458+
{
459+
startBarrier.SignalAndWait();
460+
Tools.ParallelAdd(ref total, valueToAdd);
461+
finishBarrier.SignalAndWait();
462+
}
463+
},
464+
CancellationToken.None,
465+
TaskCreationOptions.LongRunning,
466+
TaskScheduler.Default);
467+
468+
Task[] workers =
469+
{
470+
createWorker(0d),
471+
createWorker(0d),
472+
createWorker(1d),
473+
createWorker(1d)
474+
};
475+
476+
for (int round = 0; round < rounds; round++)
477+
{
478+
total = negativeZero;
479+
startBarrier.SignalAndWait();
480+
finishBarrier.SignalAndWait();
481+
if (total != 2d) everyUpdateCommitted = false;
482+
}
483+
484+
Task.WaitAll(workers);
485+
}
486+
487+
Assert.IsTrue(everyUpdateCommitted, "A finite update was lost during signed-zero contention.");
488+
}
489+
377490
/// <summary>
378491
/// Testing the Log-sum-exponential function.
379492
/// <para>

0 commit comments

Comments
 (0)