Skip to content

remove allocation in ReferenceHelper.ParseReference#617

Merged
appel1 merged 3 commits into
ExcelDataReader:developfrom
OwnageIsMagic:remove_alloc_in_ParseReference
Dec 19, 2022
Merged

remove allocation in ReferenceHelper.ParseReference#617
appel1 merged 3 commits into
ExcelDataReader:developfrom
OwnageIsMagic:remove_alloc_in_ParseReference

Conversation

@OwnageIsMagic

@OwnageIsMagic OwnageIsMagic commented Oct 27, 2022

Copy link
Copy Markdown
Contributor

On my workload allocation of substring creates significant memory pressure.

Memory profile

image

@appel1

appel1 commented Oct 29, 2022

Copy link
Copy Markdown
Collaborator

Perhaps we should consider adding a netstandard2.1 target and use TryPase that take a span, and use this implementation for older targets. @andersnm What do you think?

@appel1

appel1 commented Dec 18, 2022

Copy link
Copy Markdown
Collaborator

Looks like it is better to use this custom parse even when TryParse(ReadOnlySpan) is available.

BenchmarkDotNet=v0.13.2, OS=Windows 11 (10.0.22621.963)
Intel Core i7-8700 CPU 3.20GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET SDK=7.0.101
  [Host]     : .NET 6.0.12 (6.0.1222.56807), X64 RyuJIT AVX2
  DefaultJob : .NET 6.0.12 (6.0.1222.56807), X64 RyuJIT AVX2


|            Method | CellReference |      Mean |     Error |    StdDev |   Gen0 | Allocated |
|------------------ |-------------- |----------:|----------:|----------:|-------:|----------:|
| TryParseSubstring |            A1 | 21.705 ns | 0.1448 ns | 0.1284 ns | 0.0038 |      24 B |
|      TryParseSpan |            A1 | 14.145 ns | 0.0459 ns | 0.0407 ns |      - |         - |
|    CustomTryParse |            A1 |  9.091 ns | 0.0154 ns | 0.0144 ns |      - |         - |
| TryParseSubstring |    XFD1048576 | 26.705 ns | 0.0767 ns | 0.0718 ns | 0.0063 |      40 B |
|      TryParseSpan |    XFD1048576 | 20.319 ns | 0.0438 ns | 0.0410 ns |      - |         - |
|    CustomTryParse |    XFD1048576 | 13.920 ns | 0.0296 ns | 0.0277 ns |      - |         - |
| TryParseSubstring |            Z9 | 20.672 ns | 0.0866 ns | 0.0810 ns | 0.0038 |      24 B |
|      TryParseSpan |            Z9 | 14.135 ns | 0.0073 ns | 0.0057 ns |      - |         - |
|    CustomTryParse |            Z9 |  9.065 ns | 0.0175 ns | 0.0164 ns |      - |         - |

@OwnageIsMagic

OwnageIsMagic commented Dec 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Why it shouldn't? Look at this monster https://github.com/dotnet/runtime/blob/62013d86608b1f8923bf9bd901bb4c8ced7b20a9/src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs#L641-L817
if we roughly estimate each if is 3 ops (load, cmp, jmp) on 3 GHz cpu (3 cycles per nanosec), we can see that branch predictor and uop pipeline manage this synthetic test quite good.

@appel1 appel1 merged commit 97abcba into ExcelDataReader:develop Dec 19, 2022
@dependabot dependabot Bot mentioned this pull request Jun 10, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@OwnageIsMagic @appel1