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); + } + } +}