[API Proposal]: MemoryExtensions.Replace with separate source/destination buffers

[stephentoub]

Background and motivation

We recently added:

public static void Replace<T>(this Span<T> span, T oldValue, T newValue) where T : IEquatable<T>?

and we've been able to use it ourselves in several places, e.g. in StringBuilder.Replace. However, there are several places that actually want to be able to perform a copy/replace operation together, and doing so as a copy followed by a replace can be significantly slower (upwards of 2x) than a combined operation. For example, string.Replace needs to copy into a separate buffer and perform the replace in that buffer, and https://github.com/dotnet/aspnetcore/blob/287d7eab1c644bce2d4c96751bc3d323362b1bb7/src/Shared/QueryStringEnumerable.cs#L166-L229 similarly copies from a ReadOnlySpan<char> to a Span<char> doing a replace in the process.

We already have the implementation for this as part of string.Replace, so this is effectively just exposing that through an additional Replace overload, which can then be used in other places, like in ASP.NET.

API Proposal

namespace System

public static class MemoryExtensions
{
     public static void Replace<T>(this Span<T> span, T oldValue, T newValue) where T : IEquatable<T>?
+    public static void Replace<T>(this ReadOnlySpan<T> source, Span<T> destination, T oldValue, T newValue) where T : IEquatable<T>?
}

API Usage

// Create a string from the ReadOnlySpan<char>, replacing all pluses with spaces.
return string.Create(source.Length, (IntPtr)(&source), (dest, rosPtr) =>
{
    ReadOnlySpan<char> source = *(ReadOnlySpan<char>*)rosPtr;
    source.Replace(dest, '+', ' ');
});

Alternative Designs

No response

Risks

No response

[ghost]

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

Issue Details

---

Background and motivation

We recently added:

public static void Replace<T>(this Span<T> span, T oldValue, T newValue) where T : IEquatable<T>?

and we've been able to use it ourselves in several places. However, there are several places that actually want to be able to perform a copy/replace operation together. For example, string.Replace needs to copy into a separate buffer and perform the replace in that buffer, and https://github.com/dotnet/aspnetcore/blob/287d7eab1c644bce2d4c96751bc3d323362b1bb7/src/Shared/QueryStringEnumerable.cs#L166-L229 similarly copies from a ReadOnlySpan<char> to a Span<char> doing a replace in the process.

We already have the implementation for this as part of string.Replace, so this is effectively just exposing that through an additional Replace overload, which can then be used in other places, like in ASP.NET.

API Proposal

namespace System

public static class MemoryExtensions
{
     public static void Replace<T>(this Span<T> span, T oldValue, T newValue) where T : IEquatable<T>?
+    public static void Replace<T>(this ReadOnlySpan<T> span, Span<T>, T oldValue, T newValue) where T : IEquatable<T>?
}

API Usage

// Create a string from the ReadOnlySpan<char>, replacing all pluses with spaces.
return string.Create(source.Length, (IntPtr)(&source), (dest, rosPtr) =>
{
    ReadOnlySpan<char> source = *(ReadOnlySpan<char>*)rosPtr;
    source.Replace(dest, '+', ' ');
});

Alternative Designs

No response

Risks

No response

Author:	stephentoub
Assignees:	-
Labels:	api-suggestion, area-System.Memory
Milestone:	8.0.0

[MihaZupan]

Replace(this ReadOnlySpan span, Span, T oldValue, T newValue)

Likely meant Replace<T>(this ReadOnlySpan<T> span, Span<T> destination, T oldValue, T newValue)?

[stephentoub]

Fixed

[bartonjs]

Video

Looks good as proposed. We discussed whether or not the method warranted an "-Into" suffix, given the existing overload, and decided it didn't.

The method probably should reject source and destination in a partial overlap.

Given that the "existing" Replace method was added in this release, consider whether it is still warranted, or should be removed in favor of this one.

The concern was also raised that a this Span<T> source, Span<T> destination overload might be desired, given the extension methods. Such an overload is an easy approval of just squaring off the overload group, but isn't specifically being recommended at this time.

namespace System

public static partial class MemoryExtensions
{
     public static void Replace<T>(this ReadOnlySpan<T> source, Span<T> destination, T oldValue, T newValue) where T : IEquatable<T>?
}