Add support for ISpanFormattable in StringBuilder.Append(Object)

[ghost]

Currently, StringBuilder.Append(Object) is calling Object.ToString to get a string representation of the object passed as a parameter:

runtime/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs

Line 1081 in 1972683

public StringBuilder Append(object? value) => (value == null) ? this : Append(value.ToString());

Can we consider adding support for objects that implement ISpanFormattable here instead of calling ToString for them? This will allow to write directly to the underlying buffer rather than allocate a temporary string.

Currently to avoid the temporary string when appending an object, we can use the overload Append(ref AppendInterpolatedStringHandler) but it's not intuitive to use an interpolated string just to append an object like:

Version version = new() { Major = 2, Minor = 3, Patch = 140 };
var sb = new StringBuilder();
sb.Append(version); // ToString will be called here and a temporary string will be allocated
sb.Append($"{version}"); // ISpanFormattable.TryFormat will be called here
var result = sb.ToString();

public struct Version : ISpanFormattable
{
    const int Int32NumberBufferLength = 10 + 1; // 10 for the longest input: 2,147,483,647. We need 1 additional byte for the terminating null

    public int Major { get; init; }
    public int Minor { get; init; }
    public int Patch { get; init; }

    public override string ToString()
    {
        Span<char> destination = stackalloc char[(3 * Int32NumberBufferLength) + 3]; // at most 3 Int32s and 3 periods
        _ = TryFormatCore(destination, out int charsWritten);
        return destination.Slice(0, charsWritten).ToString();
    }

    public string ToString(string? format, IFormatProvider? formatProvider)
    {
        return ToString();
    }

    public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider)
    {
        return TryFormatCore(destination, out charsWritten);
    }

    private bool TryFormatCore(Span<char> destination, out int charsWritten)
    {
        return destination.TryWrite($"{Major}.{Minor}.{Patch}", out charsWritten);
    }
}

Benchmark:

[MemoryDiagnoser]
[HideColumns("Error", "StdDev", "Median", "RatioSD")]
public class Bench
{
    private readonly Version _version = new() { Major = 2, Minor = 3, Patch = 140 };
 
    [Benchmark]
    public string AppendObject()
    {
        var sb = new StringBuilder();
        sb.Append(_version);
        return sb.ToString();
    }

    [Benchmark]
    public string AppendInterpolated()
    {
        var sb = new StringBuilder();
        sb.Append($"{_version}");
        return sb.ToString();
    }
}

Method	Mean	Gen0	Allocated
AppendObject	78.03 ns	0.1032	216 B
AppendInterpolated	45.64 ns	0.0688	144 B

[ghost]

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

Issue Details

---

Currently, StringBuilder.Append(Object) is calling Object.ToString to get a string representation of the object passed as a parameter:

runtime/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs

Line 1081 in 1972683

public StringBuilder Append(object? value) => (value == null) ? this : Append(value.ToString());

Can we consider adding support for objects that implement ISpanFormattable here instead of calling ToString for them? This will allow to write directly to he underlying buffer rather than needing to allocate a temporary string.

Currently to avoid the temporary string when appending an object, we can use the overload Append(ref AppendInterpolatedStringHandler) but it's not intuitive to use an interpolated string just to append an object like:

Version version = new() { Major = 2, Minor = 3, Patch = 140 };
var sb = new StringBuilder();
sb.Append(Version); // ToString will be called here and a temporary string will be allocated
sb.Append($"{version}"); // ISpanFormattable.TryFormat will be called here
var result = sb.ToString();

public struct Version : ISpanFormattable
{
    const int Int32NumberBufferLength = 10 + 1; // 10 for the longest input: 2,147,483,647. We need 1 additional byte for the terminating null

    public int Major { get; init; }
    public int Minor { get; init; }
    public int Patch { get; init; }

    public override string ToString()
    {
        Span<char> destination = stackalloc char[(3 * Int32NumberBufferLength) + 3]; // at most 3 Int32s and 3 periods
        _ = TryFormatCore(destination, out int charsWritten);
        return destination.Slice(0, charsWritten).ToString();
    }

    public string ToString(string? format, IFormatProvider? formatProvider)
    {
        return ToString();
    }

    public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider)
    {
        return TryFormatCore(destination, out charsWritten);
    }

    private bool TryFormatCore(Span<char> destination, out int charsWritten)
    {
        return destination.TryWrite($"{Major}.{Minor}.{Patch}", out charsWritten);
    }
}

Benchmark:

[MemoryDiagnoser]
[HideColumns("Error", "StdDev", "Median", "RatioSD")]
public class Bench
{
    private readonly Version _version = new() { Major = 2, Minor = 3, Patch = 140 };
 
    [Benchmark]
    public string AppendObject()
    {
        var sb = new StringBuilder();
        sb.Append(_version);
        return sb.ToString();
    }

    [Benchmark]
    public string AppendInterpolated()
    {
        var sb = new StringBuilder();
        sb.Append($"{_version}");
        return sb.ToString();
    }
}

Method	Mean	Gen0	Allocated
AppendObject	78.03 ns	0.1032	216 B
AppendInterpolated	45.64 ns	0.0688	144 B

Author:	cakescience
Assignees:	-
Labels:	area-System.Runtime, untriaged
Milestone:	-

[manandre]

Good catch!
The optimization is already correctly applied for AppendFormat.
We should apply it to Append in a similar way.
Implemented in #92197

[stephentoub]

Per #92197 (comment)

[julealgon]

Can someone elaborate what goes into the decision of optimizing an existing method with a type check (like this), vs just providing an overload with the specific interface instead, such as:

public StringBuilder Append(ISpanFormattable value)

Or:

public StringBuilder Append<TSpanFormattable>(TSpanFormattable value)
    where TSpanFormattable : ISpanFormattable

Wouldn't the overload approach be more "idiomatic"/"less magical" than the internal type-check based optimization? What is the reason to not do something like that?

[MichalPetryka]

What is the reason to not do something like that?

I assume the only concern would be with bloating the API surface for something that's effectively an implementation detail.