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.

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

Change string.Repeat to use SpanAction string.Create #72

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Unordinal
Copy link

This change minimizes string.Repeat allocations via the SpanAction string.Create overload and gives a slight speed boost.

This implementation of the string repeat extension uses the string.Create overload that takes a SpanAction to write directly into the resulting string buffer instead of allocating a new string. This shaves off a bit of time and reduces the allocations by half due to not having to create a temporary buffer.

Method N Mean Error StdDev Ratio RatioSD Gen 0 Gen 1 Gen 2 Allocated
RepeatLinq 1 19.02 ns 0.214 ns 0.179 ns 1.00 0.00 0.0038 - - 40 B
RepeatStrBuilder 1 24.51 ns 0.314 ns 0.278 ns 1.29 0.02 0.0107 - - 112 B
RepeatArray 1 13.78 ns 0.102 ns 0.091 ns 0.73 0.01 0.0061 - - 64 B
RepeatSpan 1 14.55 ns 0.036 ns 0.030 ns 0.77 0.01 0.0061 - - 64 B
RepeatStringCreate 1 12.01 ns 0.070 ns 0.065 ns 0.63 0.01 0.0031 - - 32 B
RepeatLinq 10 100.08 ns 1.158 ns 1.083 ns 1.00 0.00 0.0122 - - 128 B
RepeatStrBuilder 10 62.08 ns 0.395 ns 0.330 ns 0.62 0.01 0.0213 - - 224 B
RepeatArray 10 51.17 ns 0.442 ns 0.414 ns 0.51 0.01 0.0168 - - 176 B
RepeatSpan 10 41.59 ns 0.441 ns 0.412 ns 0.42 0.01 0.0168 - - 176 B
RepeatStringCreate 10 36.51 ns 0.245 ns 0.217 ns 0.37 0.00 0.0084 - - 88 B
RepeatLinq 100 763.41 ns 4.178 ns 3.908 ns 1.00 0.00 0.0629 - - 664 B
RepeatStrBuilder 100 445.93 ns 3.844 ns 3.407 ns 0.58 0.01 0.1235 0.0005 - 1296 B
RepeatArray 100 443.01 ns 5.675 ns 5.308 ns 0.58 0.01 0.1192 - - 1248 B
RepeatSpan 100 325.17 ns 6.301 ns 6.471 ns 0.42 0.01 0.1192 - - 1248 B
RepeatStringCreate 100 288.81 ns 1.724 ns 1.613 ns 0.38 0.00 0.0596 - - 624 B
RepeatLinq 1000 6,973.50 ns 28.335 ns 26.505 ns 1.00 0.00 0.5722 0.0076 - 6064 B
RepeatStrBuilder 1000 4,266.35 ns 38.939 ns 36.423 ns 0.61 0.01 1.1520 0.0381 - 12096 B
RepeatArray 1000 4,393.29 ns 75.649 ns 77.686 ns 0.63 0.01 1.1520 0.0381 - 12048 B
RepeatSpan 1000 3,174.67 ns 31.122 ns 25.988 ns 0.46 0.00 1.1520 0.0229 - 12048 B
RepeatStringCreate 1000 2,751.88 ns 13.235 ns 11.733 ns 0.39 0.00 0.5760 0.0114 - 6024 B

@ardalis
Copy link
Owner

ardalis commented Nov 21, 2023

In your PR does it make sense to just replace Repeat with your span implementation, rather than having two versions?

@Unordinal
Copy link
Author

Unordinal commented Nov 21, 2023

In your PR does it make sense to just replace Repeat with your span implementation, rather than having two versions?

I did replace the current StringManipulationExtensions.Repeat(this string text, uint n) that uses just span copying with my implementation that uses string.Create and span copying.

Or do you mean replacing the benchmark of the current version (RepeatSpan) completely with the one from this PR (RepeatStringCreate)?

Edit: Ah, or maybe you meant having both the current version and the one from my PR? They both result in the same thing, so I don't think it would make sense to keep both.

@ardalis
Copy link
Owner

ardalis commented Nov 21, 2023

Oh if you did then never mind - I was just looking at it in the browser and saw there were 2 public methods.

@Unordinal
Copy link
Author

Oh if you did then never mind - I was just looking at it in the browser and saw there were 2 public methods.

Ah, I actually had to double-check myself when I saw that as well, hah. The new RepeatStringCreate extension in the benchmarks class is superfluous and could be removed by just using the new extension implementation directly in the benchmark method, but I thought it made some sense to separate it out for consistency with the others. I can remove it if wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants