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

Reduce some allocations with the magic of spans etc. #5938

Merged
merged 7 commits into from May 5, 2021

Conversation

@cvium
Copy link
Member

@cvium cvium commented Apr 30, 2021

Changes
euh

@cvium
Copy link
Member Author

@cvium cvium commented Apr 30, 2021

Benchmark of the SpanSplit thingy. It's not the fastest of the options I tried, but it's significantly easier to use.

|         Method |                 text |      Mean |    Error |   StdDev |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|--------------- |--------------------- |----------:|---------:|---------:|-------:|------:|------:|----------:|
|          Split | loooo(...)21321 [72] | 410.51 ns | 6.895 ns | 6.450 ns | 0.0916 |     - |     - |     768 B |
|        SpanFor | loooo(...)21321 [72] | 162.00 ns | 1.547 ns | 1.371 ns | 0.0381 |     - |     - |     320 B |
|      SpanWhile | loooo(...)21321 [72] | 147.66 ns | 1.445 ns | 1.207 ns | 0.0381 |     - |     - |     320 B |
| SpanSplitLines | loooo(...)21321 [72] | 170.97 ns | 2.388 ns | 2.117 ns | 0.0381 |     - |     - |     320 B |
|          Split |           short=1234 |  77.07 ns | 0.678 ns | 0.634 ns | 0.0162 |     - |     - |     136 B |
|        SpanFor |           short=1234 |  36.98 ns | 0.424 ns | 0.376 ns | 0.0076 |     - |     - |      64 B |
|      SpanWhile |           short=1234 |  33.96 ns | 0.638 ns | 0.566 ns | 0.0076 |     - |     - |      64 B |
| SpanSplitLines |           short=1234 |  40.08 ns | 0.373 ns | 0.331 ns | 0.0076 |     - |     - |      64 B |
public static LineSplitEnumerator Split(this ReadOnlySpan<char> str, char separator) => new (str, separator);

[StructLayout(LayoutKind.Auto)]
public ref struct LineSplitEnumerator

This comment has been minimized.

@Bond-009

Bond-009 May 1, 2021
Member

This isn't a LineSplitEnumerator anymore

This comment has been minimized.

@cvium

cvium May 1, 2021
Author Member

True. I'm terrible with names though

This comment has been minimized.

@Bond-009

Bond-009 May 2, 2021
Member

Just SplitEnumerator?

@Bond-009 Bond-009 merged commit 48e81e6 into jellyfin:master May 5, 2021
14 checks passed
14 checks passed
@github-actions
main main
Details
@github-actions
Analyze (csharp) Analyze (csharp)
Details
@github-actions
triage
Details
@github-code-scanning
CodeQL No new or fixed alerts
Details
@azure-pipelines
Jellyfin Server Build #20210504.10 succeeded
Details
@azure-pipelines
Jellyfin Server (Build Debug) Build Debug succeeded
Details
@azure-pipelines
Jellyfin Server (Build Release) Build Release succeeded
Details
@azure-pipelines
Jellyfin Server (Compatibility Check Common) Compatibility Check Common succeeded
Details
@azure-pipelines
Jellyfin Server (Compatibility Check Controller) Compatibility Check Controller succeeded
Details
@azure-pipelines
Jellyfin Server (Compatibility Check Model) Compatibility Check Model succeeded
Details
@azure-pipelines
Jellyfin Server (Compatibility Check Naming) Compatibility Check Naming succeeded
Details
@azure-pipelines
Jellyfin Server (Test Linux) Test Linux succeeded
Details
@azure-pipelines
Jellyfin Server (Test Windows) Test Windows succeeded
Details
@azure-pipelines
Jellyfin Server (Test macOS) Test macOS succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants