Let a compiled expression be called from more than one thread (#637) - #698
Conversation
…mmunity#637) FastExpression kept its working stack and its subexpression cache as instance fields, so two threads calling one compiled expression interleaved their pushes and pops. The count check at the end then found a stack it did not leave, and threw. Measured with sixteen threads over 400000 calls of one expression: 399328 threw "Unused values remain in the stack", 418 threw "Stack empty", and one returned -0.483 where the answer is 14884.07. The wrong number is the part worth naming -- the report only saw the exception, and a wrong number is the worse of the two. The damage was also permanent. The count check throws before anything is popped, so a single racing call left its leftovers on the stack and every later call failed on them too, on one thread or on many. Both buffers are now per thread rather than per expression, in one object so that the lookup happens once. Per thread and not per call because allocating nothing per call is the point of compiling. Neither buffer carries anything between calls: the stack is emptied on the way in, and a cache slot is only ever read by the same call that wrote it, which is how the compiler emits them -- SAVE_CACHE at the definition, LOAD_CACHE at each later use. Nothing is slower for it. Best of five runs, single-threaded: x^2 + 3x + 1 44.6 -> 43.6 ns/call sin(u)cos(u) + u^2 + sin(u), u = x^2+1 123.4 -> 118.8 ns/call Compiling in parallel, which is what the issue title says, was never broken; there is a test for it now so that it stays that way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #698 +/- ##
==========================================
- Coverage 80.99% 80.57% -0.42%
==========================================
Files 155 156 +1
Lines 13687 12924 -763
Branches 1957 2124 +167
==========================================
- Hits 11086 10414 -672
+ Misses 1990 1906 -84
+ Partials 611 604 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
CI, run on my fork across the full matrix, since the run here is held at https://github.com/Rafael-SOWNet/AngouriMath/actions/runs/30930183903
Worth having all three here: the new tests run 16 threads over 300 000 calls, and thread scheduling is exactly the sort of thing that differs by platform. The reporter is on The branch it ran on, |
Fixes #637.
Reproduced, and it is worse than reported
FastExpressionkept its working stack and its subexpression cache as instance fields:so two threads calling one compiled expression interleave their pushes and pops. Sixteen threads, 400 000 calls of a single expression:
AngouriBugException: Unused values remain in the stackInvalidOperationException: Stack empty.The wrong number is the part worth naming. It returned
-0.483where the answer is14884.07. The report only saw the exception, and of the two that is the kinder failure.The damage was also permanent. The count check throws before anything is popped, so one racing call left its leftovers on the stack and every later call tripped over them — including single-threaded ones afterwards:
Compiling in parallel — what the issue title says — was never broken. 2000 independent compilations across 16 threads: no failures, before or after. It is calling that was not safe.
The fix
Both buffers are now per thread rather than per expression, held in one small object so the thread-static lookup happens once per call.
Per thread and not per call because allocating nothing per call is the point of compiling. Neither buffer carries anything between calls:
SAVE_CACHEat the definition of a repeated subexpression,LOAD_CACHEat each later use, always in that order (IntoFE/Compiler.cs,InnerCompile).It is not slower
Best of five runs each, single-threaded, warmed:
x^2 + 3x + 1sin(u)cos(u) + u^2 + sin(u),u = x^2+1(My first measurement was a single unwarmed run and showed a 33% regression. It was noise; these are best-of-five and the difference is within it either way. Recording the wrong first number here because it is the sort of thing worth not believing.)
Tests
Sources/Tests/UnitTests/Common/ParallelCompiledCallTest.cs, 4 cases. 3 fail without the source change (verified by reverting it in place):Full suite:
Failed: 0, Passed: 4013, Skipped: 14, Total: 4027.Not covered
This makes one compiled expression safe to call concurrently. It does not make
MathS.Settingsor the rest of the library's per-thread state safe to share, and it does not touch the Linq-compiled path, which builds aFunc<...>and has no such state to begin with.🤖 Generated with Claude Code