Remove unsafe code from number parsing and formatting - #131913
Draft
tannergooding wants to merge 3 commits into
Draft
Remove unsafe code from number parsing and formatting#131913tannergooding wants to merge 3 commits into
tannergooding wants to merge 3 commits into
Conversation
Replace pointer-based parsing and formatting with span-based implementations while preserving direct writes and hot-path performance. 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. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes remaining pointer-based/unsafe parsing and formatting across core number/date/time codepaths by switching to Span-based digit buffers and helper routines, aiming to keep the same output while improving safety and maintaining performance.
Changes:
- Replaces
byte*/fixed-based digit walking and writing withReadOnlySpan<byte>/Span<TChar>implementations (including new two-digit writers and direct string-span initialization). - Refactors several formatting/parsing helpers (e.g.,
TryParseNumber,DigitsToUInt32/UInt64, BigInteger formatting) to avoid pinning and pointer arithmetic. - Updates DateTime/TimeSpan formatting routines to use span slices and builder copy patterns instead of raw pointers.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Runtime.Numerics/src/System/Number.Polyfill.cs | Simplifies UTF-8/UTF-16 decode branches and span casts. |
| src/libraries/System.Runtime.Numerics/src/System/Number.BigInteger.cs | Removes unsafe from BigInteger parse/format paths; introduces span-based digit writing helpers. |
| src/libraries/System.Private.CoreLib/src/System/Numerics/Decimal64.cs | Switches significand extraction from pointer to digit span slicing. |
| src/libraries/System.Private.CoreLib/src/System/Numerics/Decimal32.cs | Switches significand extraction from pointer to digit span slicing. |
| src/libraries/System.Private.CoreLib/src/System/Numerics/Decimal128.cs | Switches significand extraction from pointer to digit span slicing. |
| src/libraries/System.Private.CoreLib/src/System/Number.Rounding.cs | Uses stackalloc-backed NumberBuffer span constructor instead of pointer-based buffer. |
| src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs | Converts internal integer/decimal/float parsing helpers to span-based digit access. |
| src/libraries/System.Private.CoreLib/src/System/Number.NumberToFloatingPointBits.cs | Replaces pointer-based digit parsing with span slicing and BinaryPrimitives-based reads. |
| src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs | Reworks integer/decimal/float formatting to avoid pointers; adds direct writable string spans and digit-pair optimizations. |
| src/libraries/System.Private.CoreLib/src/System/Number.DecimalIeee754.cs | Replaces Unsafe.SizeOf<T> with sizeof(T) for exponent bit computations. |
| src/libraries/System.Private.CoreLib/src/System/Number.BigInteger.cs | Addresses scoped ref-safety and out-param constraints; avoids full buffer zeroing via SkipInit. |
| src/libraries/System.Private.CoreLib/src/System/Globalization/TimeSpanFormat.cs | Removes pointer writes; writes prefix/suffix via span slices. |
| src/libraries/System.Private.CoreLib/src/System/Globalization/NumberFormatInfo.cs | Simplifies UTF-8 cache span creation using AsSpan. |
| src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs | Removes unsafe from ParseByFormat signature. |
| src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeFormat.cs | Removes pointer writes; uses spans/builders for formatting. |
| src/libraries/Common/src/System/Number.Parsing.Common.cs | Rewrites core number parsing from pointer range to span+index scanning. |
| src/libraries/Common/src/System/Number.NumberBuffer.cs | Makes NumberBuffer non-unsafe; removes DigitsPtr and pointer-based ctor. |
| src/libraries/Common/src/System/Number.Formatting.Common.cs | Adds span-based two-digit writers/tables and converts formatting helpers away from pointers. |
Copilot's findings
- Files reviewed: 18/18 changed files
- Comments generated: 2
EgorBo
reviewed
Aug 6, 2026
| 0x00300037u, 0x00310037u, 0x00320037u, 0x00330037u, 0x00340037u, 0x00350037u, 0x00360037u, 0x00370037u, 0x00380037u, 0x00390037u, | ||
| 0x00300038u, 0x00310038u, 0x00320038u, 0x00330038u, 0x00340038u, 0x00350038u, 0x00360038u, 0x00370038u, 0x00380038u, 0x00390038u, | ||
| 0x00300039u, 0x00310039u, 0x00320039u, 0x00330039u, 0x00340039u, 0x00350039u, 0x00360039u, 0x00370039u, 0x00380039u, 0x00390039u, | ||
| ]; |
Member
There was a problem hiding this comment.
it's not possible to keep the previous readable strings/utf8 literals?
private static ReadOnlySpan<byte> TwoDigitsCharsAsBytes =>
MemoryMarshal.AsBytes<char>("00010203040506070809" +
"10111213141516171819" +
"20212223242526272829" +
"30313233343536373839" +
"40414243444546474849" +
"50515253545556575859" +
"60616263646566676869" +
"70717273747576777879" +
"80818283848586878889" +
"90919293949596979899");
private static ReadOnlySpan<byte> TwoDigitsBytes =>
"00010203040506070809"u8 +
"10111213141516171819"u8 +
"20212223242526272829"u8 +
"30313233343536373839"u8 +
"40414243444546474849"u8 +
"50515253545556575859"u8 +
"60616263646566676869"u8 +
"70717273747576777879"u8 +
"80818283848586878889"u8 +
"90919293949596979899"u8;Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (1)
src/libraries/Common/src/System/Number.NumberBuffer.cs:36
- The XML doc for the
digitsparameter still states the span must refer to unmovable memory (stack / pinned), butNumberBufferno longer stores a raw pointer (DigitsPtrwas removed) and all accesses are span-based. Callers can now safely pass spans backed by movable arrays (e.g.,ArrayPoolrentals), so this documentation is misleading/outdated.
/// <param name="digits">The digits scratch space. The referenced memory must not be moveable, e.g. stack memory, pinned array, etc.</param>
- Files reviewed: 18/18 changed files
- Comments generated: 0 new
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces pointer-based parsing and formatting with span-based implementations across
NumberBuffer, numeric types,DateTime,TimeSpan, decimal floating-point types, andBigInteger.The replacement paths preserve direct writes while avoiding additional allocations, temporary copies, and per-element destination bounds checks.
Performance
Ratio is candidate / baseline; lower is better.
0.86-0.91x0.91-0.95x0.71-0.78xRformatting0.66x0.94-0.97x24%fasterD/X/BformatsUInt128 Bremained within approximately1%noise1.10x1.03-1.07xThere were no allocation increases in the 82-benchmark matrix.
The remaining low-level operations are narrow, explicitly audited exceptions:
ToStringusesFastAllocateStringand creates a writable span over the fresh exact-length string. The string cannot escape before every character is initialized.Unsafe.BitCastoperations.MemoryMarshal.Writeand a two-byte-aligned packed digit-pair representation to retain one store per pair without pointer arithmetic.Unsafe.SkipInitpaths where all consumed elements are initialized before being read.BigIntegerarithmetic reinterpretations remain where their sizes and representations are fixed.No executable
unsafe, pointer writers, pinning, orfixedstatements remain in the affected paths.The final baseline/candidate harness produced byte-identical output for 2,266 cases.
System.Runtime.Testspassed 76,641 tests andSystem.Runtime.Numerics.Testspassed 8,422 tests.Fixes #10397
Note
This description was drafted by Copilot.