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

Optimize ReplaceLineEndings #81630

Merged
merged 2 commits into from
Feb 20, 2023
Merged

Optimize ReplaceLineEndings #81630

merged 2 commits into from
Feb 20, 2023

Conversation

MihaZupan
Copy link
Member

  • If we haven't replaced anything, avoid allocating the new string
    • Loop in IndexOfNewlineChar until something other than the replacementText is found
  • If we're replacing new lines with a line feed ('\n' - default on non-Windows), we can avoid searching for the line feed, thus avoiding breaking out of the vectorized path unnecessarily
    • If we're searching for 5 values instead of 6, we also switch from the probabilistic map to SpanHelpers.IndexOfAnyValueType

ReplaceLineEndings LF => LF

Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0 10000 LF LF 11,532.5 ns 1.00 -
pr 0 10000 LF LF 622.6 ns 0.05 -
main 0.05 10000 LF LF 22,552.8 ns 1.00 20024 B
pr 0.05 10000 LF LF 607.1 ns 0.03 -
main 0.1 10000 LF LF 32,663.1 ns 1.00 20024 B
pr 0.1 10000 LF LF 614.8 ns 0.02 -
main 0.2 10000 LF LF 50,579.0 ns 1.00 20024 B
pr 0.2 10000 LF LF 621.8 ns 0.01 -
main 1 10000 LF LF 129,070.5 ns 1.000 20024 B
pr 1 10000 LF LF 607.5 ns 0.005 -
ReplaceLineEndings CRLF => LF
Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0 10000 CRLF LF 11,528.7 ns 1.00 -
pr 0 10000 CRLF LF 623.5 ns 0.05 -
main 0.05 10000 CRLF LF 21,548.3 ns 1.00 19040 B
pr 0.05 10000 CRLF LF 8,531.8 ns 0.40 19040 B
main 0.1 10000 CRLF LF 28,978.2 ns 1.00 18128 B
pr 0.1 10000 CRLF LF 17,105.9 ns 0.59 18128 B
main 0.2 10000 CRLF LF 41,437.0 ns 1.00 16664 B
pr 0.2 10000 CRLF LF 30,343.2 ns 0.73 16664 B
main 1 10000 CRLF LF 67,488.9 ns 1.00 10024 B
pr 1 10000 CRLF LF 58,969.6 ns 0.87 10024 B
ReplaceLineEndings CRLF => CRLF
Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0 10000 CRLF CRLF 11.65 us 1.00 -
pr 0 10000 CRLF CRLF 11.25 us 0.96 -
main 0.05 10000 CRLF CRLF 23.29 us 1.00 20024 B
pr 0.05 10000 CRLF CRLF 16.64 us 0.71 -
main 0.1 10000 CRLF CRLF 32.72 us 1.00 20024 B
pr 0.1 10000 CRLF CRLF 20.16 us 0.62 -
main 0.2 10000 CRLF CRLF 47.12 us 1.00 20024 B
pr 0.2 10000 CRLF CRLF 28.28 us 0.60 -
main 1 10000 CRLF CRLF 79.54 us 1.00 20024 B
pr 1 10000 CRLF CRLF 35.94 us 0.45 -
SpanLineEnumerator
Method Toolchain NewLineFrequency Length Mean Error Ratio
EnumerateLines main 0 10000 11.52 us 0.046 us 1.00
EnumerateLines pr 0 10000 11.36 us 0.049 us 0.99
EnumerateLines main 0.05 10000 18.35 us 0.080 us 1.00
EnumerateLines pr 0.05 10000 17.70 us 0.082 us 0.97
EnumerateLines main 0.1 10000 26.19 us 0.093 us 1.00
EnumerateLines pr 0.1 10000 23.65 us 0.077 us 0.90
EnumerateLines main 0.2 10000 44.13 us 0.320 us 1.00
EnumerateLines pr 0.2 10000 37.83 us 0.153 us 0.86
EnumerateLines main 1 10000 122.93 us 0.679 us 1.00
EnumerateLines pr 1 10000 83.27 us 0.224 us 0.68

@MihaZupan MihaZupan added this to the 8.0.0 milestone Feb 4, 2023
@MihaZupan MihaZupan self-assigned this Feb 4, 2023
@ghost
Copy link

ghost commented Feb 4, 2023

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

Issue Details
  • If we haven't replaced anything, avoid allocating the new string
    • Loop in IndexOfNewlineChar until something other than the replacementText is found
  • If we're replacing new lines with a line feed ('\n' - default on non-Windows), we can avoid searching for the line feed, thus avoiding breaking out of the vectorized path unnecessarily
    • If we're searching for 5 values instead of 6, we also switch from the probabilistic map to SpanHelpers.IndexOfAnyValueType

ReplaceLineEndings LF => LF

Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0 10000 LF LF 11,532.5 ns 1.00 -
pr 0 10000 LF LF 622.6 ns 0.05 -
main 0.05 10000 LF LF 22,552.8 ns 1.00 20024 B
pr 0.05 10000 LF LF 607.1 ns 0.03 -
main 0.1 10000 LF LF 32,663.1 ns 1.00 20024 B
pr 0.1 10000 LF LF 614.8 ns 0.02 -
main 0.2 10000 LF LF 50,579.0 ns 1.00 20024 B
pr 0.2 10000 LF LF 621.8 ns 0.01 -
main 1 10000 LF LF 129,070.5 ns 1.000 20024 B
pr 1 10000 LF LF 607.5 ns 0.005 -
ReplaceLineEndings CRLF => LF
Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0 10000 CRLF LF 11,528.7 ns 1.00 -
pr 0 10000 CRLF LF 623.5 ns 0.05 -
main 0.05 10000 CRLF LF 21,548.3 ns 1.00 19040 B
pr 0.05 10000 CRLF LF 8,531.8 ns 0.40 19040 B
main 0.1 10000 CRLF LF 28,978.2 ns 1.00 18128 B
pr 0.1 10000 CRLF LF 17,105.9 ns 0.59 18128 B
main 0.2 10000 CRLF LF 41,437.0 ns 1.00 16664 B
pr 0.2 10000 CRLF LF 30,343.2 ns 0.73 16664 B
main 1 10000 CRLF LF 67,488.9 ns 1.00 10024 B
pr 1 10000 CRLF LF 58,969.6 ns 0.87 10024 B
ReplaceLineEndings CRLF => CRLF
Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0 10000 CRLF CRLF 11.65 us 1.00 -
pr 0 10000 CRLF CRLF 11.25 us 0.96 -
main 0.05 10000 CRLF CRLF 23.29 us 1.00 20024 B
pr 0.05 10000 CRLF CRLF 16.64 us 0.71 -
main 0.1 10000 CRLF CRLF 32.72 us 1.00 20024 B
pr 0.1 10000 CRLF CRLF 20.16 us 0.62 -
main 0.2 10000 CRLF CRLF 47.12 us 1.00 20024 B
pr 0.2 10000 CRLF CRLF 28.28 us 0.60 -
main 1 10000 CRLF CRLF 79.54 us 1.00 20024 B
pr 1 10000 CRLF CRLF 35.94 us 0.45 -
SpanLineEnumerator
Method Toolchain NewLineFrequency Length Mean Error Ratio
EnumerateLines main 0 10000 11.52 us 0.046 us 1.00
EnumerateLines pr 0 10000 11.36 us 0.049 us 0.99
EnumerateLines main 0.05 10000 18.35 us 0.080 us 1.00
EnumerateLines pr 0.05 10000 17.70 us 0.082 us 0.97
EnumerateLines main 0.1 10000 26.19 us 0.093 us 1.00
EnumerateLines pr 0.1 10000 23.65 us 0.077 us 0.90
EnumerateLines main 0.2 10000 44.13 us 0.320 us 1.00
EnumerateLines pr 0.2 10000 37.83 us 0.153 us 0.86
EnumerateLines main 1 10000 122.93 us 0.679 us 1.00
EnumerateLines pr 1 10000 83.27 us 0.224 us 0.68
Author: MihaZupan
Assignees: MihaZupan
Labels:

area-System.Runtime
Milestone: 8.0.0

@MihaZupan
Copy link
Member Author

Comparing the combined change of #81630 + #80963 + #78678 against main without them (NET 7 vs 8):

ReplaceLineEndings LF => LF

Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0.05 10000 LF LF 28,130.2 ns 1.00 20024 B
pr 0.05 10000 LF LF 623.1 ns 0.02 -
main 0.1 10000 LF LF 45,561.1 ns 1.00 20024 B
pr 0.1 10000 LF LF 607.1 ns 0.01 -
main 0.2 10000 LF LF 79,684.7 ns 1.000 20024 B
pr 0.2 10000 LF LF 607.1 ns 0.008 -
main 1 10000 LF LF 252,398.5 ns 1.000 20024 B
pr 1 10000 LF LF 620.9 ns 0.002 -
ReplaceLineEndings CRLF => LF
Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0.05 10000 CRLF LF 26,852.0 ns 1.00 19040 B
pr 0.05 10000 CRLF LF 8,702.7 ns 0.32 19040 B
main 0.1 10000 CRLF LF 41,837.3 ns 1.00 18128 B
pr 0.1 10000 CRLF LF 16,860.2 ns 0.40 18128 B
main 0.2 10000 CRLF LF 67,020.5 ns 1.00 16664 B
pr 0.2 10000 CRLF LF 29,707.1 ns 0.44 16664 B
main 1 10000 CRLF LF 136,184.9 ns 1.00 10024 B
pr 1 10000 CRLF LF 58,194.7 ns 0.43 10024 B
ReplaceLineEndings CRLF => CRLF
Toolchain NewLineFrequency Length Source Replacement Mean Ratio Allocated
main 0.05 10000 CRLF CRLF 28.686 us 1.00 20024 B
pr 0.05 10000 CRLF CRLF 6.715 us 0.23 -
main 0.1 10000 CRLF CRLF 44.628 us 1.00 20024 B
pr 0.1 10000 CRLF CRLF 11.373 us 0.25 -
main 0.2 10000 CRLF CRLF 70.777 us 1.00 20024 B
pr 0.2 10000 CRLF CRLF 18.566 us 0.26 -
main 1 10000 CRLF CRLF 148.458 us 1.00 20024 B
pr 1 10000 CRLF CRLF 54.425 us 0.37 -
SpanLineEnumerator
Method Toolchain NewLineFrequency Length Mean Error Ratio
EnumerateLines main 0 10000 8.558 us 0.0333 us 1.00
EnumerateLines pr 0 10000 1.091 us 0.0108 us 0.13
EnumerateLines main 0.05 10000 23.230 us 0.0779 us 1.00
EnumerateLines pr 0.05 10000 8.074 us 0.0991 us 0.35
EnumerateLines main 0.1 10000 37.880 us 0.1584 us 1.00
EnumerateLines pr 0.1 10000 13.635 us 0.0982 us 0.36
EnumerateLines main 0.2 10000 67.683 us 0.3634 us 1.00
EnumerateLines pr 0.2 10000 26.506 us 0.1376 us 0.39
EnumerateLines main 1 10000 241.015 us 1.0229 us 1.00
EnumerateLines pr 1 10000 125.364 us 1.4975 us 0.52

@MihaZupan
Copy link
Member Author

Any thoughts on this @dotnet/area-system-runtime?

stephentoub
stephentoub reviewed Feb 17, 2023
View reviewed changes
src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
Comment on lines +1499 to +1513
var builder = new ValueStringBuilder(stackalloc char[StackallocCharBufferSizeLimit]);
while (true)
{
int idx = remaining.IndexOfAny(IndexOfAnyValuesStorage.NewLineCharsExceptLineFeed);
if ((uint)idx >= (uint)remaining.Length) break; // no more newline chars
stride = remaining[idx] == '\r' && (uint)(idx + 1) < (uint)remaining.Length && remaining[idx + 1] == '\n' ? 2 : 1;
builder.Append('\n');
builder.Append(remaining.Slice(0, idx));
remaining = remaining.Slice(idx + stride);
}

return idx;
builder.Append('\n');
string retVal = Concat(this.AsSpan(0, idxOfFirstNewlineChar), builder.AsSpan(), remaining);
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, I wonder what the performance profile would look like for instead doing the equivalent of a Count("\r\n"), then allocating a string of exactly the right length (string.Length - count), and then formatting directly into that string, rather than going into a growable VSB and then copying into a new string at the end.

Copy link
Member Author

Choose a reason for hiding this comment

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

Given this is effectively a glorified string.Replace, we could use a similar approach of building the list of indices.
I'll try it out.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tested with MihaZupan@b68c952 and it's similar or a bit worse across the board.

@build-analysis build-analysis bot mentioned this pull request Feb 20, 2023
Closed
@MihaZupan MihaZupan merged commit b698ad8 into dotnet:main Feb 20, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Miizukii Miizukii Miizukii left review comments

@stephentoub stephentoub stephentoub approved these changes

Assignees

@MihaZupan MihaZupan

Labels
area-System.Runtime
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@MihaZupan @stephentoub @Miizukii