Skip to content

Use Span.Fill in ValueStringBuilder.Append for repeating chars - #131652

Open
prozolic wants to merge 6 commits into
dotnet:mainfrom
prozolic:valuestringbuilder-append
Open

prozolic wants to merge 6 commits into
dotnet:mainfrom
prozolic:valuestringbuilder-append

Conversation

@prozolic

Copy link
Copy Markdown
Contributor

This PR uses Span<char>.Fill in ValueStringBuilder.Append(char c, int count) and ValueStringBuilder<TChar>.Append(TChar c, int count) instead of a for loop to add each char.
This change is expected to improve performance for larger sizes.

Benchmark

The benchmark uses BigInteger.ToString/BigInteger.TryFormat, which internally use ValueStringBuilder<TChar>.Append(TChar c, int count)

Method Job Toolchain Padding Mean Error StdDev Median Min Max Ratio RatioSD Gen0 Allocated Alloc Ratio
ToString_Binary Job-QIDXWP \main\corerun.exe 1 53.49 ns 4.516 ns 5.201 ns 52.80 ns 45.88 ns 62.27 ns 1.00 0.00 0.0049 32 B 1.00
ToString_Binary Job-TLPBEL \pr\corerun.exe 1 47.55 ns 2.400 ns 2.668 ns 47.47 ns 42.59 ns 53.49 ns 0.90 0.10 0.0049 32 B 1.00
TryFormat_Binary Job-QIDXWP \main\corerun.exe 1 38.48 ns 1.970 ns 2.108 ns 37.83 ns 35.55 ns 43.40 ns 1.00 0.00 - - NA
TryFormat_Binary Job-TLPBEL \pr\corerun.exe 1 38.83 ns 1.274 ns 1.308 ns 38.71 ns 35.83 ns 40.48 ns 1.01 0.06 - - NA
ToString_Binary Job-QIDXWP \main\corerun.exe 4 51.56 ns 3.178 ns 3.532 ns 50.35 ns 47.13 ns 61.45 ns 1.00 0.00 0.0063 40 B 1.00
ToString_Binary Job-TLPBEL \pr\corerun.exe 4 48.15 ns 1.408 ns 1.506 ns 48.16 ns 45.64 ns 50.94 ns 0.94 0.07 0.0063 40 B 1.00
TryFormat_Binary Job-QIDXWP \main\corerun.exe 4 39.45 ns 2.126 ns 2.363 ns 39.62 ns 34.10 ns 43.53 ns 1.00 0.00 - - NA
TryFormat_Binary Job-TLPBEL \pr\corerun.exe 4 38.37 ns 1.308 ns 1.399 ns 38.49 ns 35.64 ns 41.50 ns 0.98 0.07 - - NA
ToString_Binary Job-QIDXWP \main\corerun.exe 16 58.34 ns 3.190 ns 3.413 ns 58.62 ns 53.47 ns 65.93 ns 1.00 0.00 0.0101 64 B 1.00
ToString_Binary Job-TLPBEL \pr\corerun.exe 16 52.59 ns 2.213 ns 2.368 ns 52.64 ns 47.66 ns 56.74 ns 0.90 0.06 0.0100 64 B 1.00
TryFormat_Binary Job-QIDXWP \main\corerun.exe 16 41.96 ns 1.108 ns 1.138 ns 42.10 ns 39.02 ns 43.52 ns 1.00 0.00 - - NA
TryFormat_Binary Job-TLPBEL \pr\corerun.exe 16 37.81 ns 1.457 ns 1.496 ns 37.60 ns 35.27 ns 40.15 ns 0.90 0.04 - - NA
ToString_Binary Job-QIDXWP \main\corerun.exe 64 76.80 ns 3.734 ns 3.995 ns 75.83 ns 71.39 ns 84.62 ns 1.00 0.00 0.0252 160 B 1.00
ToString_Binary Job-TLPBEL \pr\corerun.exe 64 59.92 ns 3.232 ns 3.459 ns 59.79 ns 55.52 ns 68.02 ns 0.78 0.06 0.0253 160 B 1.00
TryFormat_Binary Job-QIDXWP \main\corerun.exe 64 59.51 ns 6.173 ns 6.605 ns 59.50 ns 51.53 ns 75.67 ns 1.00 0.00 - - NA
TryFormat_Binary Job-TLPBEL \pr\corerun.exe 64 39.51 ns 1.829 ns 1.878 ns 40.05 ns 35.84 ns 42.44 ns 0.67 0.07 - - NA
ToString_Binary Job-QIDXWP \main\corerun.exe 256 179.44 ns 5.598 ns 5.749 ns 178.89 ns 165.36 ns 191.94 ns 1.00 0.00 0.0861 544 B 1.00
ToString_Binary Job-TLPBEL \pr\corerun.exe 256 86.34 ns 4.322 ns 4.977 ns 85.90 ns 77.26 ns 95.18 ns 0.48 0.03 0.0867 544 B 1.00
TryFormat_Binary Job-QIDXWP \main\corerun.exe 256 130.84 ns 5.883 ns 6.539 ns 128.21 ns 123.39 ns 149.16 ns 1.00 0.00 - - NA
TryFormat_Binary Job-TLPBEL \pr\corerun.exe 256 42.79 ns 2.006 ns 2.230 ns 43.23 ns 38.15 ns 46.16 ns 0.33 0.02 - - NA
Benchmark source
    [BenchmarkCategory(Categories.Libraries)]
    public class Perf_FormatPaddingForBigInteger
    {
        private const int CharsForBits = 2;

        [Params(1, 4, 16, 64, 256)]
        public int Padding;

        private BigInteger bigInteger;
        private string paddedFormat;
        private char[] destination;

        [GlobalSetup]
        public void Setup()
        {
            bigInteger = BigInteger.One;
            paddedFormat = $"B{(CharsForBits + Padding)}";
            destination = new char[CharsForBits + Padding];
        }

        [Benchmark]
        public string ToString_Binary()
        {
            return bigInteger.ToString(paddedFormat);
        }

        [Benchmark]
        public bool TryFormat_Binary()
        {
            return bigInteger.TryFormat(destination, out _, paddedFormat);
        }
    }

This PR uses Span<char>.Fill in ValueStringBuilder.Append(char c, int
count) and ValueStringBuilder<TChar>.Append(TChar c, int count)
instead of a for loop to add each char.
This change expected to improve performance for larger sizes.
Copilot AI lite review requested due to automatic review settings July 31, 2026 16:16
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jul 31, 2026
@azure-pipelines

Copy link
Copy Markdown
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.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces manual per-element loops with Span<T>.Fill when appending a repeated character into ValueStringBuilder buffers, leveraging the framework’s optimized fill implementations to improve performance on larger repeat counts.

Changes:

  • Update ValueStringBuilder.Append(char c, int count) to use _chars.Slice(...).Fill(c) instead of a for loop.
  • Update ValueStringBuilder<TChar>.Append(TChar c, int count) to use _chars.Slice(...).Fill(c) instead of a for loop.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/libraries/Common/src/System/Text/ValueStringBuilder.cs Replaces the repeated-char append loop with Span<char>.Fill.
src/libraries/Common/src/System/Text/ValueStringBuilder_1.cs Replaces the repeated-char append loop with Span<TChar>.Fill in the generic builder.

@prozolic
prozolic marked this pull request as ready for review July 31, 2026 16:23
@azure-pipelines

Copy link
Copy Markdown
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.

@tannergooding tannergooding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. CC. @jeffhandley for whether the trivial 2 line perf improvement is ok to take into .NET 11

Copilot AI review requested due to automatic review settings August 13, 2026 00:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings September 14, 2026 13:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved issues were identified, and behavior is preserved.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@prozolic

prozolic commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

@jeffhandley Gentle ping. Is there anything else I should do before this can be merged?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Runtime community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants