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

Avoid extra allocations in YamlScalarNode GetHashCode #648

Merged
merged 1 commit into from
Nov 15, 2021

Conversation

lahma
Copy link
Contributor

@lahma lahma commented Nov 13, 2021

YamlScalarNode calls CombineHashCodes(Tag, Value) which is the only call in library that chooses the params array method. No need to allocate array for this so changed method signature to take the second object as parameter.

EDIT

Then found out that the first parameter is actuall struct (TagName Tag) so causes boxing, is wiser to call hash code directly and now remove the CombineHashCodes overload that is no longer needed at all.

Also marked TagName as read-only struct. Should also use ThrowHelper here, but didn't duplicate introduction work from other PR.

Benchmark Code

Uses the large YAML from issue #519 . I noticed you don't have a benchmark project currently so I'm running my local one.

[MemoryDiagnoser]
public class YamlStreamBenchmark
{
    private string yamlString = "";

    [GlobalSetup]
    public void Setup()
    {
        yamlString = File.ReadAllText(@"c:\work\saltern.yml");
    }

    [Benchmark]
    public void Load()
    {
        var yamlStream = new YamlStream();
        yamlStream.Load(new StringReader(yamlString));
    }
}

Results

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.22000
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=6.0.100
  [Host]     : .NET 6.0.0 (6.0.21.52210), X64 RyuJIT
  DefaultJob : .NET 6.0.0 (6.0.21.52210), X64 RyuJIT

YamlStreamBenchmark

Diff Method Mean Error Gen 0 Gen 1 Gen 2 Allocated
Old Load 96.60 ms 1.183 ms 5833.3333 2833.3333 1000.0000 81 MB
New 95.72 ms (-1%) 1.659 ms 5666.6667 (-3%) 2500.0000 (-12%) 1000.0000 (0%) 78 MB (-4%)

@lahma lahma force-pushed the avoid-params-array-in-hash-code branch from f1476bc to d2599e8 Compare November 13, 2021 09:41
@lahma lahma changed the title Avoid params array construction in CombineHashCodes Avoid extra allocations in YamlScalarNode GetHashCode Nov 13, 2021
@aaubry aaubry merged commit 0d145bd into aaubry:master Nov 15, 2021
@lahma lahma deleted the avoid-params-array-in-hash-code branch November 15, 2021 10:12
@aaubry
Copy link
Owner

aaubry commented Jul 23, 2022

This feature has been released in version 12.0.0.

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