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

Add fast path to MemoryExtensions.Trim for input that needs no trimming #84210

Merged
merged 8 commits into from
Apr 2, 2023
Merged

Add fast path to MemoryExtensions.Trim for input that needs no trimming #84210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1b92cf1
Improve perf when input doesn't contain whitespace + better perf for …
EgorBo Apr 1, 2023
45a9f05
Update Guid.cs
EgorBo Apr 1, 2023
3e0708e
Address feedback
EgorBo Apr 1, 2023
72f8224
Revert previous changes
EgorBo Apr 1, 2023
d966449
Update src/libraries/System.Private.CoreLib/src/System/MemoryExtensio…
EgorBo Apr 1, 2023
202e283
Address feedback
EgorBo Apr 1, 2023
7df3319
Apply suggestions from code review
EgorBo Apr 1, 2023
a5c2cbe
Address feedback
EgorBo Apr 1, 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
2 changes: 1 addition & 1 deletion src/libraries/System.Private.CoreLib/src/System/Guid.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ public static bool TryParseExact([NotNullWhen(true)] string? input, [NotNullWhen

public static bool TryParseExact(ReadOnlySpan<char> input, [StringSyntax(StringSyntaxAttribute.GuidFormat)] ReadOnlySpan<char> format, out Guid result)
{
if (format.Length != 1)
if (format.Length != 1 || input.Length < 32) // Minimal length we can parse ('N' format)
{
result = default;
return false;
Expand Down
71 changes: 56 additions & 15 deletions src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.Trim.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Runtime.CompilerServices;

namespace System
{
Expand Down Expand Up @@ -565,27 +566,39 @@ public static ReadOnlyMemory<char> TrimEnd(this ReadOnlyMemory<char> memory)
/// Removes all leading and trailing white-space characters from the span.
/// </summary>
/// <param name="span">The source span from which the characters are removed.</param>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static ReadOnlySpan<char> Trim(this ReadOnlySpan<char> span)
{
int start = 0;
for (; start < span.Length; start++)
// Assume that in most cases input doesn't need trimming
if (span.Length == 0 ||
(!char.IsWhiteSpace(span[0]) && !char.IsWhiteSpace(span[^1])))
{
if (!char.IsWhiteSpace(span[start]))
{
break;
}
return span;
}
return TrimFallback(span);

EgorBo marked this conversation as resolved.
Show resolved Hide resolved
int end = span.Length - 1;
for (; end > start; end--)
[MethodImpl(MethodImplOptions.NoInlining)]
static ReadOnlySpan<char> TrimFallback(ReadOnlySpan<char> span)
{
if (!char.IsWhiteSpace(span[end]))
int start = 0;
for (; start < span.Length; start++)
{
break;
if (!char.IsWhiteSpace(span[start]))
{
break;
}
}
}

return span.Slice(start, end - start + 1);
int end = span.Length - 1;
for (; end > start; end--)
{
if (!char.IsWhiteSpace(span[end]))
{
break;
}
}
return span.Slice(start, end - start + 1);
}
}

/// <summary>
Expand Down Expand Up @@ -770,11 +783,39 @@ public static ReadOnlySpan<char> TrimEnd(this ReadOnlySpan<char> span, ReadOnlyS
/// Removes all leading and trailing white-space characters from the span.
/// </summary>
/// <param name="span">The source span from which the characters are removed.</param>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Span<char> Trim(this Span<char> span)
{
int start = ClampStart(span);
int length = ClampEnd(span, start);
return span.Slice(start, length);
// Assume that in most cases input doesn't need trimming
if (span.Length == 0 ||
(!char.IsWhiteSpace(span[0]) && !char.IsWhiteSpace(span[^1])))
{
return span;
}
return TrimFallback(span);

[MethodImpl(MethodImplOptions.NoInlining)]
static Span<char> TrimFallback(Span<char> span)
{
int start = 0;
for (; start < span.Length; start++)
{
if (!char.IsWhiteSpace(span[start]))
{
break;
}
}
EgorBo marked this conversation as resolved.
Show resolved Hide resolved

int end = span.Length - 1;
for (; end > start; end--)
{
if (!char.IsWhiteSpace(span[end]))
{
break;
}
}
return span.Slice(start, end - start + 1);
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the clamp helpers in the fallback?

Copy link
Member Author

@EgorBo EgorBo Apr 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Since we add an additional overhead for cases when input does contain whitespaces I decided to have a slightly faster fallback (inlined clamp helpers) - didn't want to mark the helpers always-inlineable. Can revert to clamp helpers if you think an extra 1ns is not worth it

}
}

/// <summary>
Expand Down