Uh oh!
There was an error while loading. Please reload this page.
Remove the Number static constructor - #132550
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Tagging subscribers to this area: @dotnet/area-system-numerics |
tannergooding
commented
Aug 20, 2026
This was raised by @MichalStrehovsky in MichalStrehovsky/rt-sz#247 as caused by #131019 Basically the issue was some new static readonly field introduced. So this PR fully removes all the static fields, ensuring we either use RVA statics or a helper lookup so that it can be fully avoided and there is now no |
There was a problem hiding this comment.
Pull request overview
Refactors System.Number implementation details to eliminate the type’s static constructor by moving previously eagerly-initialized caches/tables into RVA-backed spans and nested holder types, computing the remaining small constants on demand. This primarily targets improved trimming (unused data not rooted by .cctor) while preserving existing formatting/math behavior.
Changes:
- Replaces managed string-array formatting patterns with switch-selected
ReadOnlySpan<byte>format templates in the shared formatting helper. - Moves small number and Mono “TwoDigits” caches behind nested holder types to avoid
Number-level eager initialization. - Converts multiple Decimal IEEE754 coefficient/constant tables (trig/log/pow/exp/hyper) from
static readonlyarrays to span-backed representations plus small on-demand constant getters.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs | Moves small-number and Mono TwoDigits caches into nested holders to avoid Number static initialization. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.Transcendental.cs | Updates call sites to use on-demand pi / inverse-trig constants instead of static arrays. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128Trig.cs | Switches trig coefficient tables to span-backed coefficient data. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128Sqrt.cs | Replaces the sqrt coefficient struct table with split span-backed AB/C coefficient data. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128Pow.cs | Converts pow coefficient tables to span-backed coefficients. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128PiTrig.cs | Replaces static pi-fraction constants with an on-demand getter and makes small constants computed properties. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128Log.cs | Converts log coefficient table to span-backed coefficients. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128InvTrig.cs | Replaces inverse-trig constants array with an on-demand getter; converts coefficient tables to span-backed coefficients. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128Hyper.cs | Converts sinh/cosh coefficient tables to span-backed coefficients. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128Exp.cs | Adds a shared span-casting helper for fixed coefficients and converts exp/exp10 tables to span-backed coefficients. |
| src/libraries/Common/src/System/Number.Formatting.Common.cs | Replaces currency/number/percent format string arrays with switch-selected ReadOnlySpan<byte> templates. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 0
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.DiyFp128Exp.cs:39
DiyFp128FixedCoefficientsrelies onMemoryMarshal.Cast<ulong, DiyFp128FixedCoefficient>which assumesDiyFp128FixedCoefficientstays exactly 2 ulongs (16 bytes). Adding an explicit size assertion makes this helper robust against future edits to the struct that would otherwise silently reinterpret data incorrectly.
private static ReadOnlySpan<DiyFp128FixedCoefficient> DiyFp128FixedCoefficients(ReadOnlySpan<ulong> coefficients)
{
Debug.Assert((coefficients.Length & 1) == 0);
return MemoryMarshal.Cast<ulong, DiyFp128FixedCoefficient>(coefficients);
}
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
Uh oh!
There was an error while loading. Please reload this page.
EgorBo
commented
Aug 20, 2026
Uh oh!
There was an error while loading. Please reload this page.
MichalStrehovsky
commented
Aug 20, 2026
Looks like this fully fixes the regression: MichalStrehovsky/rt-sz#249
|
Uh oh!
There was an error while loading. Please reload this page.
tannergooding
commented
Aug 20, 2026
Failures were fixed by #132549, rerunning since this hasn't gotten approval yet |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
tannergooding
commented
Aug 20, 2026
/backport to release/11.0 |
Started backporting to |
Moves
Number's primitive Decimal32/64/128 coefficient data to RVA-backed spans and computes the remaining small structured constants on demand. Managed caches that still require initialization are isolated in nested holder types, soNumberno longer has a static constructor and unused data can be trimmed.Formatting patterns now use switch-selected RVA byte spans rather than managed string arrays. Direct byte handling was within benchmark noise versus the prior implementation: currency
-1.9%, number-1.2%, and percent+0.2%, with unchanged allocations.Validated CoreCLR, NativeAOT, and Mono builds and all 77,366
System.Runtime.Tests.Note
This pull request was created with GitHub Copilot.