From 22c1bdf1557a71cb10f808e8388a13b6637af7ae Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Sun, 9 Aug 2026 18:56:20 +0000 Subject: [PATCH] Hold a settings scope per flow instead of per thread MathS.Settings kept its fourteen values in [ThreadStatic] fields, so a scope stopped at the first await: using var _ = MathS.Settings.MaxExpansionTermCount.Set(1); // 1 await Task.Delay(20).ConfigureAwait(false); // 2000 -- the default, silently The continuation resumed on a pool thread that had never seen the scope. The same mechanism ran the other way: a thread going back to the pool still carried whatever scope was left on it, so the next caller to borrow it could compute under a precision or codomain it never asked for. Neither surfaces as an error; both change the answer. The obvious repair -- AsyncLocal> on the static field -- is wrong. An AsyncLocal flows the reference, and Setting was mutable, so two concurrent tasks would have pushed onto one shared stack. What is per-flow has to be the value stack itself, so the stack moved inside Setting as an immutable chain behind an AsyncLocal, and the static fields became ordinary singletons. They also had to stop being lazily created: `field ??= default` on a non-thread-static field races, and a scope opened on the loser of that race would vanish. Scopes are identified by a counter rather than by their frame. Releasing a scope that is not on top rebuilds the frames above it, and a rebuilt frame is a different object, so keying on the frame left everything above it impossible to release afterwards -- which the out-of-order test caught. Dropping the Guid this replaces is most of why Set() got cheaper. Measured, 20M reads and 2M scopes: read PrecisionErrorZeroRange.Value 7.3-8.0 ns -> 1.2 ns read DowncastingEnabled.Value 0.79 ns -> 0.96 ns Set() + Dispose() 388-395 ns, 32 B -> 46 ns, 112 B a Simplify workload 5.6-5.8 s -> 5.5-5.6 s, +1.5% allocated Reads got faster because the old getter re-tested a [ThreadStatic] field for null on every access. Opening a scope allocates more, since assigning an AsyncLocal copies the flow's value map, but reads outnumber scope openings by orders of magnitude. The recursion-depth counters and per-thread scratch caches elsewhere keep [ThreadStatic] deliberately: an lHopital depth must not follow a call into a sibling. RewriteRecording has the same per-thread ambient shape and is left alone here; its documentation already says "on this thread". Verified: 6069 C# tests and 130 F# tests pass, and the public surface is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- BREAKING-CHANGES.md | 56 +++++++ Sources/.editorconfig | 3 + .../Diagnostic/MathS.Diagnostic.cs | 6 +- Sources/AngouriMath/Convenience/MathS.cs | 39 ++--- .../AngouriMath/Convenience/SettingClass.cs | 129 ++++++++++------ .../Multithreading/SettingsAcrossAsync.cs | 145 ++++++++++++++++++ 6 files changed, 301 insertions(+), 77 deletions(-) create mode 100644 Sources/Tests/UnitTests/Core/Multithreading/SettingsAcrossAsync.cs diff --git a/BREAKING-CHANGES.md b/BREAKING-CHANGES.md index 8bcf7515d..b6be1bfaa 100644 --- a/BREAKING-CHANGES.md +++ b/BREAKING-CHANGES.md @@ -58,6 +58,7 @@ read first. | loud | the target frameworks | `net7.0;netstandard2.0` | `netstandard2.0;net8.0;net10.0` | | **silent** | `abs(x) = c` for a negative `c` | a set of non-solutions | the empty set | | loud | implicit `List` to `Entity` | made a `FiniteSet`, and made three `params` overloads uncallable | removed | +| **silent** | a `MathS.Settings` scope across an `await`, or inside a task | lost, or somebody else's | follows the call | --- @@ -149,6 +150,61 @@ Entity set = new Entity[] { 1, 2, 3 }; // the array conversion is unchange The `Entity[]` conversion is kept: an array argument binds to the `params` overload in its normal form by an identity conversion, which beats the alternatives outright, so it never produced the ambiguity. +### A settings scope belongs to the call, not to the thread + +`MathS.Settings` values were held in `[ThreadStatic]` fields — fourteen of them. A scope +therefore stopped at the first `await`: + +```csharp +using var _ = MathS.Settings.MaxExpansionTermCount.Set(1); +Console.WriteLine(MathS.Settings.MaxExpansionTermCount.Value); // 1 +await Task.Delay(20).ConfigureAwait(false); +Console.WriteLine(MathS.Settings.MaxExpansionTermCount.Value); // was 2000, the default +``` + +The continuation resumed on a pool thread that had never seen the scope, so the setting +silently read its default. The same mechanism ran the other way too: a thread returned to +the pool still carried whatever scope was left on it, so the *next* caller to borrow that +thread could compute under a precision or codomain it never asked for. Neither shows up as +an error; both change the answer. + +The values now live in an `AsyncLocal`, which is what the cancellation token in +`MathS.Multithreading` already used. + +| | was | is | +|---|---|---| +| a scope across an `await` | lost | kept | +| a scope inside `Task.Run` started under it | not inherited | **inherited** | +| a scope opened in a task, seen by a sibling | no | no | +| a scope opened in a task, seen after it ends | no | no | +| a thread reused by the pool | could carry a stale scope | cannot | + +**What breaks.** The second row is the one to read. Work started inside a scope now runs +under it: + +```csharp +using var _ = MathS.Settings.Codomain.Set(Domain.Real); +await Task.Run(() => expr.Solve("x")); // now solves over R; used to solve over C +``` + +That is what the code says, and almost always what was meant — but if you parallelised +inside a scope and relied on the child *not* seeing it, it does now. Move the scope inside +the callback to keep the old behaviour. + +**Cost.** Measured over 20 000 000 reads and 2 000 000 scopes: + +| | was | is | +|---|---|---| +| read `PrecisionErrorZeroRange.Value` | 7.3–8.0 ns | **1.2 ns** | +| read `DowncastingEnabled.Value` | 0.79 ns | 0.96 ns | +| `Set()` + `Dispose()` | 388–395 ns, 32 B | **46 ns**, 112 B | +| a `Simplify` workload | 5.6–5.8 s | 5.5–5.6 s, +1.5 % allocated | + +Reads got faster rather than slower: the old getter re-tested a `[ThreadStatic]` field for +null on every access, and that is dearer than the async-local lookup that replaced it. +Opening a scope got much cheaper because it no longer mints a `Guid` to identify itself, +though it allocates more, since assigning an `AsyncLocal` copies the flow's value map. Reads +outnumber scope openings by orders of magnitude in any real workload. ### `Minusf`'s two operands exchanged names diff --git a/Sources/.editorconfig b/Sources/.editorconfig index 08be865e7..860684a90 100644 --- a/Sources/.editorconfig +++ b/Sources/.editorconfig @@ -27,6 +27,9 @@ file_header_template=\nCopyright (c) 2019-2026 Angouri.\nAngouriMath is licensed [Tests/UnitTests/Common/ListArgumentOverloadTest.cs] file_header_template=\nCopyright (c) 2019-2026 Angouri.\nAngouriMath is licensed under MIT.\nDetails: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.\nWebsite: https://am.angouri.org.\n +[Tests/UnitTests/Core/Multithreading/SettingsAcrossAsync.cs] +file_header_template=\nCopyright (c) 2019-2026 Angouri.\nAngouriMath is licensed under MIT.\nDetails: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.\nWebsite: https://am.angouri.org.\n + # A new file in a directory whose other files predate it, so the section is on the file # rather than the folder. [AngouriMath/Core/Entity/Continuous/Entity.Continuous.{Floors,Rounding}.Classes.cs] diff --git a/Sources/AngouriMath/Convenience/Diagnostic/MathS.Diagnostic.cs b/Sources/AngouriMath/Convenience/Diagnostic/MathS.Diagnostic.cs index 745f8f783..38f37b31b 100644 --- a/Sources/AngouriMath/Convenience/Diagnostic/MathS.Diagnostic.cs +++ b/Sources/AngouriMath/Convenience/Diagnostic/MathS.Diagnostic.cs @@ -22,16 +22,14 @@ public static class Diagnostic /// Explicit output for ToString, that is, no signs or parentheses will be omitted. Useful /// for debugging and diagnostic. /// - public static Setting OutputExplicit => outputExplicit ??= false; - [ThreadStatic] private static Setting? outputExplicit; + public static Setting OutputExplicit { get; } = false; /// /// Set a predicate on the state of so that once /// the predicate turns true in method , /// an exception is thrown. /// - public static Setting> CatchOnSimplify => catchOnSimplify ??= (Func)(a => false); - [ThreadStatic] private static Setting>? catchOnSimplify; + public static Setting> CatchOnSimplify { get; } = (Func)(a => false); /// /// Will only occur in debug mode, diff --git a/Sources/AngouriMath/Convenience/MathS.cs b/Sources/AngouriMath/Convenience/MathS.cs index 3b0b1856c..25e3952e6 100644 --- a/Sources/AngouriMath/Convenience/MathS.cs +++ b/Sources/AngouriMath/Convenience/MathS.cs @@ -5347,8 +5347,7 @@ public static partial class Settings /// Exception, but we still can parse a ^ 2 + 2 * x + a * b + 2 * (g + e) ^ 3 /// /// - public static Setting ExplicitParsingOnly => explicitParsingOnly ??= false; - [ThreadStatic] private static Setting? explicitParsingOnly; + public static Setting ExplicitParsingOnly { get; } = false; /// /// That is how we perform newton solving when no analytical solution was found @@ -5400,8 +5399,7 @@ public sealed record NewtonSetting /// AngouriMath.Entity+Number+Real /// /// - public static Setting DowncastingEnabled => downcastingEnabled ??= true; - [ThreadStatic] private static Setting? downcastingEnabled; + public static Setting DowncastingEnabled { get; } = true; /// /// Amount of iterations allowed for attempting to cast to a rational @@ -5448,8 +5446,7 @@ public sealed record NewtonSetting /// 5/7 /// /// - public static Setting FloatToRationalIterCount => floatToRationalIterCount ??= 15; - [ThreadStatic] private static Setting? floatToRationalIterCount; + public static Setting FloatToRationalIterCount { get; } = 15; /// /// If a numerator or denominator is too large, it's suspended to better keep the real number instead of casting @@ -5503,24 +5500,20 @@ public sealed record NewtonSetting /// 100/169137 /// /// - public static Setting MaxAbsNumeratorOrDenominatorValue => - maxAbsNumeratorOrDenominatorValue ??= EInteger.FromInt32(100000000); - [ThreadStatic] private static Setting? maxAbsNumeratorOrDenominatorValue; + public static Setting MaxAbsNumeratorOrDenominatorValue { get; } = EInteger.FromInt32(100000000); /// /// Sets threshold for comparison /// For example, if you don't need precision higher than 6 digits after ., /// you can set it to 1.0e-6 so 1.0000000 == 0.9999999 /// - public static Setting PrecisionErrorCommon => precisionErrorCommon ??= EDecimal.Create(1, -6); - [ThreadStatic] private static Setting? precisionErrorCommon; + public static Setting PrecisionErrorCommon { get; } = EDecimal.Create(1, -6); /// /// Numbers whose absolute value is less than PrecisionErrorZeroRange are considered zeros /// - public static Setting PrecisionErrorZeroRange => precisionErrorZeroRange ??= EDecimal.Create(1, -16); - [ThreadStatic] private static Setting? precisionErrorZeroRange; + public static Setting PrecisionErrorZeroRange { get; } = EDecimal.Create(1, -16); /// /// The tolerance within which downcasting rounds a number onto a nearby integer. @@ -5576,8 +5569,7 @@ internal static EDecimal DowncastingTolerance /// { } // nothing was found for 5-degree polynomial without numeric solution /// /// - public static Setting AllowNewton => allowNewton ??= true; - [ThreadStatic] private static Setting? allowNewton; + public static Setting AllowNewton { get; } = true; /// /// Criteria for simplifier so you could control which expressions are considered easier by you. @@ -5649,8 +5641,7 @@ internal static EDecimal DowncastingTolerance /// By default criteria it cannot simplify it further, however, the custom one /// it simplified from 2 to 1. /// - public static Setting> ComplexityCriteria => - complexityCriteria ??= new Func(expr => + public static Setting> ComplexityCriteria { get; } = new Func(expr => { // Those are of the 2nd power to avoid problems with floating numbers const double TinyWeight = 0.5; @@ -5688,7 +5679,6 @@ internal static EDecimal DowncastingTolerance } + Weight; // Number of nodes return DefaultCriteria(expr); }); - [ThreadStatic] private static Setting>? complexityCriteria; /// /// Settings for the Newton-Raphson's root-search method @@ -5703,8 +5693,7 @@ internal static EDecimal DowncastingTolerance /// ... /// /// - public static Setting NewtonSolver => newtonSolver ??= new NewtonSetting(); - [ThreadStatic] private static Setting? newtonSolver; + public static Setting NewtonSolver { get; } = new NewtonSetting(); /// /// The maximum number of linear children of an expression in polynomial solver @@ -5736,15 +5725,12 @@ internal static EDecimal DowncastingTolerance /// /// /// - public static Setting MaxExpansionTermCount => maxExpansionTermCount ??= 2_000; - [ThreadStatic] private static Setting? maxExpansionTermCount; + public static Setting MaxExpansionTermCount { get; } = 2_000; /// /// Settings for precisions of PeterO.Numbers /// - public static Setting DecimalPrecisionContext => - decimalPrecisionContext ??= new EContext(100, ERounding.HalfUp, -100, 1000, false); - [ThreadStatic] private static Setting? decimalPrecisionContext; + public static Setting DecimalPrecisionContext { get; } = new EContext(100, ERounding.HalfUp, -100, 1000, false); /// /// Whether functions are being read as real-valued or complex-valued. It is a @@ -5777,8 +5763,7 @@ internal static EDecimal DowncastingTolerance /// prints the unevaluated limit, where the default prints -oo. /// /// - public static Setting Codomain => codomain ??= Domain.Complex; - [ThreadStatic] private static Setting? codomain; + public static Setting Codomain { get; } = Domain.Complex; } /// Returns an in polynomial order if possible diff --git a/Sources/AngouriMath/Convenience/SettingClass.cs b/Sources/AngouriMath/Convenience/SettingClass.cs index 43de595de..f9e9a8099 100644 --- a/Sources/AngouriMath/Convenience/SettingClass.cs +++ b/Sources/AngouriMath/Convenience/SettingClass.cs @@ -1,4 +1,4 @@ -// +// // Copyright (c) 2019-2022 Angouri. // AngouriMath is licensed under MIT. // Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md. @@ -6,6 +6,7 @@ // using System; +using System.Threading; namespace AngouriMath.Convenience { @@ -15,14 +16,50 @@ namespace AngouriMath.Convenience /// /// Those configurations can be of different types /// + /// + /// + /// A setting is one object for the whole process, and what it holds is per *flow*, not + /// per thread: the stack of values lives in an . A scope + /// opened before an is therefore still in force after it, and a + /// scope opened inside a task is invisible to that task's siblings and to whatever + /// started it. Backing the field with [ThreadStatic] gave neither: a continuation + /// resumed on a pool thread saw the defaults, and a thread returned to the pool carried + /// whatever scope was left on it into the next caller. + /// + /// + /// The frames are immutable, which is what makes the reference safe to share. An + /// copies on assignment to + /// and on nothing else, so if a frame could be mutated in place, two flows holding the + /// same chain would write over each other. Pushing and popping therefore build a new + /// chain and assign it, rather than editing one. + /// + /// public sealed class Setting where T : notnull { - internal Setting(T defaultValue) + /// + /// One pushed value, and the chain below it. Never mutated once built. + /// + /// + /// is what a scope is released by, rather than the frame's own + /// reference. Releasing a scope that is not on top rebuilds everything above it, and + /// a rebuilt frame is a different object — so identifying a scope by its frame would + /// leave the ones above it impossible to release afterwards. The id is a counter and + /// not a : generating a guid per scope cost more than everything + /// else does put together. + /// + private sealed class Frame { - Set(defaultValue); - Default = defaultValue; + internal readonly long Id; + internal readonly T Value; + internal readonly Frame? Next; + internal Frame(long id, T value, Frame? next) => (Id, Value, Next) = (id, value, next); } + private readonly AsyncLocal frames = new(); + private long lastId; + + internal Setting(T defaultValue) => Default = defaultValue; + /// /// Sets the new value for the setting /// @@ -40,9 +77,42 @@ internal Setting(T defaultValue) /// public AutoBackRollableTemporarySettingUnit Set(T value) { - var guid = Guid.NewGuid(); - values.Push(guid, value); - return new AutoBackRollableTemporarySettingUnit(this, guid); + var id = Interlocked.Increment(ref lastId); + frames.Value = new Frame(id, value, frames.Value); + return new AutoBackRollableTemporarySettingUnit(this, id); + } + + /// + /// Takes the scope numbered back out of this flow's chain, + /// wherever in it it sits. Disposal is normally in the reverse order of + /// , which is the cheap case of popping the head; out-of-order + /// disposal rebuilds what was above it. An id that is not in this flow's chain — + /// because the scope was opened in another one, or is already released — is not an + /// error and undoes nothing. + /// + private void Remove(long id) + { + var top = frames.Value; + if (top is null) + return; + if (top.Id == id) + { + frames.Value = top.Next; + return; + } + var above = new System.Collections.Generic.List(); + var current = top; + while (current is not null && current.Id != id) + { + above.Add(current); + current = current.Next; + } + if (current is null) + return; + var rebuilt = current.Next; + for (var i = above.Count - 1; i >= 0; i--) + rebuilt = new Frame(above[i].Id, above[i].Value, rebuilt); + frames.Value = rebuilt; } /// @@ -92,12 +162,10 @@ public TReturnType As(T value, Func action) /// public override string? ToString() => Value.ToString(); - private readonly KeyStack values = new(); - /// /// The current value of the setting /// - public T Value => values.Peek(); + public T Value => frames.Value is { } frame ? frame.Value : Default; /// /// The default value of the setting @@ -110,7 +178,7 @@ public TReturnType As(T value, Func action) /// Lets a default be treated as "nobody expressed an opinion" rather than as a /// deliberate choice. /// - internal bool IsOverriden => values.Count > 1; + internal bool IsOverriden => frames.Value is not null; /// /// This tiny struct is needed to be under `using` operator, so that your settings @@ -124,49 +192,18 @@ public struct AutoBackRollableTemporarySettingUnit : IDisposable { private readonly Setting setting; private bool disposed; - private readonly Guid guid; - internal AutoBackRollableTemporarySettingUnit(Setting settingToRollBack, Guid guid) - => (this.setting, disposed, this.guid) = (settingToRollBack, false, guid); + private readonly long id; + internal AutoBackRollableTemporarySettingUnit(Setting settingToRollBack, long id) + => (setting, disposed, this.id) = (settingToRollBack, false, id); /// public void Dispose() { if (disposed) return; - setting.values.Remove(guid); + setting.Remove(id); disposed = true; } } } - - - internal sealed class KeyStack - { - private readonly List<(TKey key, TValue value)> list = new(); - - internal int Count => list.Count; - - internal TValue Peek() => list[^1].value; - - internal bool Remove(TKey key) - { - int index = -1; - for (int i = list.Count - 1; i >= 0; i--) - if (key is not null && key.Equals(list[i].key)) - { - index = i; - break; - } - if (index == -1) - return false; - list.RemoveAt(index); - return true; - } - - internal void Push(TKey key, TValue value) - => list.Add((key, value)); - - internal void Pop() - => list.RemoveAt(list.Count - 1); - } } diff --git a/Sources/Tests/UnitTests/Core/Multithreading/SettingsAcrossAsync.cs b/Sources/Tests/UnitTests/Core/Multithreading/SettingsAcrossAsync.cs new file mode 100644 index 000000000..fef292a03 --- /dev/null +++ b/Sources/Tests/UnitTests/Core/Multithreading/SettingsAcrossAsync.cs @@ -0,0 +1,145 @@ +// +// Copyright (c) 2019-2026 Angouri. +// AngouriMath is licensed under MIT. +// Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md. +// Website: https://am.angouri.org. +// + +using System.Threading; +using System.Threading.Tasks; +using AngouriMath; +using Xunit; + +namespace AngouriMath.Tests.Core.Multithreading +{ + /// + /// A setting scope belongs to the flow that opened it, not to the thread that happened + /// to be running it. These are the four properties that distinguishes: it survives an + /// await, it is not visible to a sibling, it does not escape upwards, and it unwinds in + /// whatever order the scopes are disposed. + /// + [Trait("Area", "Core")] + public sealed class SettingsAcrossAsync + { + static long Current => MathS.Settings.MaxExpansionTermCount.Value; + static long Default => MathS.Settings.MaxExpansionTermCount.Default; + + /// + /// The regression. Backed by [ThreadStatic] the continuation resumed on a pool + /// thread that had never seen the scope, and the setting silently read its default. + /// + [Fact] + public async Task ScopeSurvivesAnAwait() + { + Assert.NotEqual(27L, Default); + using var _ = MathS.Settings.MaxExpansionTermCount.Set(27); + Assert.Equal(27, Current); + await Task.Yield(); + Assert.Equal(27, Current); + await Task.Delay(20).ConfigureAwait(false); + Assert.Equal(27, Current); + } + + /// + /// What the per-thread field did give, and what a naive AsyncLocal<Setting<T>> + /// would have taken away: two flows sharing one setting object would have pushed onto + /// one stack. The barrier makes both scopes open before either reads. + /// + [Fact] + public async Task SiblingFlowsDoNotSeeEachOther() + { + using var barrier = new Barrier(2); + async Task Branch(long value) + { + await Task.Yield(); + using var _ = MathS.Settings.MaxExpansionTermCount.Set(value); + barrier.SignalAndWait(); + await Task.Delay(20).ConfigureAwait(false); + return Current; + } + var both = await Task.WhenAll(Branch(101), Branch(202)); + Assert.Equal(new[] { 101L, 202L }, both); + } + + /// + /// The direction of the change that can surprise someone: work started under a scope + /// runs under it. With the per-thread field the pool thread had never seen the scope + /// and the child read the default instead. + /// + [Fact] + public async Task WorkStartedUnderAScopeInheritsIt() + { + using var _ = MathS.Settings.MaxExpansionTermCount.Set(77); + Assert.Equal(77, await Task.Run(() => Current)); + Assert.Equal(77, await Task.Run(async () => { await Task.Yield(); return Current; })); + } + + /// A scope opened inside a task is gone once that task is. + [Fact] + public async Task ScopeDoesNotEscapeUpwards() + { + await Task.Run(async () => + { + using var _ = MathS.Settings.MaxExpansionTermCount.Set(55); + await Task.Yield(); + Assert.Equal(55, Current); + }); + Assert.Equal(Default, Current); + } + + [Fact] + public void NestedScopesUnwindInOrder() + { + using (var _ = MathS.Settings.MaxExpansionTermCount.Set(1)) + { + Assert.Equal(1, Current); + using (var __ = MathS.Settings.MaxExpansionTermCount.Set(2)) + Assert.Equal(2, Current); + Assert.Equal(1, Current); + } + Assert.Equal(Default, Current); + } + + /// + /// Disposal is normally the reverse of opening, so popping the head covers it. This + /// is the other path: releasing the outer scope first has to rebuild the chain above + /// it and leave the inner one standing. + /// + [Fact] + public void OutOfOrderDisposalKeepsTheOtherScope() + { + var outer = MathS.Settings.MaxExpansionTermCount.Set(7); + var inner = MathS.Settings.MaxExpansionTermCount.Set(9); + Assert.Equal(9, Current); + + outer.Dispose(); + Assert.Equal(9, Current); + + inner.Dispose(); + Assert.Equal(Default, Current); + } + + [Fact] + public void DisposingTwiceIsHarmless() + { + var scope = MathS.Settings.MaxExpansionTermCount.Set(31); + using (var other = MathS.Settings.MaxExpansionTermCount.Set(32)) + { + scope.Dispose(); + scope.Dispose(); + Assert.Equal(32, Current); + } + Assert.Equal(Default, Current); + } + + /// A setting nobody has touched reports its default and says so. + [Fact] + public void UntouchedSettingReadsItsDefault() + { + Assert.Equal(Default, Current); + using (var _ = MathS.Settings.MaxExpansionTermCount.Set(Default + 1)) + Assert.Equal(Default + 1, Current); + Assert.Equal(Default, Current); + } + } +}