Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove check from FillStringChecked and rename it to CopyStringContent #85880

Merged
merged 10 commits into from
May 9, 2023
Merged

Remove check from FillStringChecked and rename it to CopyStringContent #85880

Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
175b7e8
Remove AO from a couple of SpanHelpers methods
EgorBo May 5, 2023
d22aa95
Merge branch 'main' of github.com:dotnet/runtime
EgorBo May 6, 2023
ec7503a
Address feedback
EgorBo May 7, 2023
be05813
Merge branch 'main' of github.com:dotnet/runtime into opt-string-fill…
EgorBo May 8, 2023
959df9d
Address feedback
EgorBo May 8, 2023
f5919ac
remove [AI]
EgorBo May 8, 2023
8ff9a62
Address feedback
EgorBo May 8, 2023
f368752
Address feedback
EgorBo May 8, 2023
fdbb1ac
Merge branch 'main' of github.com:dotnet/runtime into opt-string-fill…
EgorBo May 8, 2023
49509cd
Switch to int.MaxValue
EgorBo May 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 32 additions & 18 deletions src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,31 @@ internal static class SearchValuesStorage
internal const int StackallocIntBufferSizeLimit = 128;
internal const int StackallocCharBufferSizeLimit = 256;

private static void FillStringChecked(string dest, int destPos, string src)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void CopyStringContent(string dest, int destPos, string src)
{
Debug.Assert(dest != null);
Debug.Assert(src != null);
if (src.Length > dest.Length - destPos)
{
throw new IndexOutOfRangeException();
}
Debug.Assert(src.Length <= dest.Length - destPos);

Buffer.Memmove(
destination: ref Unsafe.Add(ref dest._firstChar, destPos),
source: ref src._firstChar,
elementCount: (uint)src.Length);
}

private static void CopyStringContentChecked(string dest, int destPos, string src)
{
Debug.Assert(dest != null);
Debug.Assert(src != null);
if (src.Length > dest.Length - destPos)
{
throw new IndexOutOfRangeException();
}

CopyStringContent(dest, destPos, src);
}

public static string Concat(object? arg0) =>
arg0?.ToString() ?? Empty;

Expand Down Expand Up @@ -117,7 +127,7 @@ public static string Concat(params object?[] args)
Debug.Assert(s != null);
Debug.Assert(position <= totalLength - s.Length, "We didn't allocate enough space for the result string!");

FillStringChecked(result, position, s);
CopyStringContentChecked(result, position, s);
position += s.Length;
}

Expand Down Expand Up @@ -255,10 +265,14 @@ public static string Concat(string? str0, string? str1)

int str0Length = str0.Length;

string result = FastAllocateString(str0Length + str1.Length);
// totalLength will never overflow to a non-negative number
// and the negative number will trigger OOM in FastAllocateString.
int totalLength = str0Length + str1.Length;

string result = FastAllocateString(totalLength);

FillStringChecked(result, 0, str0);
FillStringChecked(result, str0Length, str1);
CopyStringContent(result, 0, str0);
CopyStringContent(result, str0Length, str1);

return result;
}
Expand All @@ -283,9 +297,9 @@ public static string Concat(string? str0, string? str1, string? str2)
int totalLength = str0.Length + str1.Length + str2.Length;

string result = FastAllocateString(totalLength);
FillStringChecked(result, 0, str0);
FillStringChecked(result, str0.Length, str1);
FillStringChecked(result, str0.Length + str1.Length, str2);
CopyStringContentChecked(result, 0, str0);
CopyStringContentChecked(result, str0.Length, str1);
CopyStringContentChecked(result, str0.Length + str1.Length, str2);

return result;
}
Expand Down Expand Up @@ -315,10 +329,10 @@ public static string Concat(string? str0, string? str1, string? str2, string? st
int totalLength = str0.Length + str1.Length + str2.Length + str3.Length;

string result = FastAllocateString(totalLength);
FillStringChecked(result, 0, str0);
FillStringChecked(result, str0.Length, str1);
FillStringChecked(result, str0.Length + str1.Length, str2);
FillStringChecked(result, str0.Length + str1.Length + str2.Length, str3);
CopyStringContentChecked(result, 0, str0);
CopyStringContentChecked(result, str0.Length, str1);
CopyStringContentChecked(result, str0.Length + str1.Length, str2);
CopyStringContentChecked(result, str0.Length + str1.Length + str2.Length, str3);

return result;
}
Expand Down Expand Up @@ -470,7 +484,7 @@ public static string Concat(params string?[] values)
break;
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}

FillStringChecked(result, copiedLength, value);
CopyStringContentChecked(result, copiedLength, value);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
copiedLength += valueLen;
}
}
Expand Down Expand Up @@ -970,7 +984,7 @@ private static string JoinCore(ReadOnlySpan<char> separator, ReadOnlySpan<string
}
EgorBo marked this conversation as resolved.
Show resolved Hide resolved

// Fill in the value.
FillStringChecked(result, copiedLength, value);
CopyStringContentChecked(result, copiedLength, value);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
copiedLength += valueLen;
}

Expand Down