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

[C#] Make DirectBuffer.CheckLimit inlinable #840

Merged
merged 3 commits into from
Mar 31, 2021

Conversation

szehetner
Copy link
Contributor

DirectBuffer.CheckLimit(int limit) is called in a lot of places during encoding and decoding and would profit from being inlined by the JIT Compiler. Currently it can't because of the throw statements and it's size. This change extracts the cold path where the check fails into a separate method, making CheckLimit itself eligible for inlining and resulting in a measureable performance improvement.

Running the existing benchmark project:
Before:

##teamcity[buildStatisticValue key='AverageEncodeLatencyNanos' value='40,0']
##teamcity[buildStatisticValue key='AverageDecodeLatencyNanos' value='33,8']

After:

##teamcity[buildStatisticValue key='AverageEncodeLatencyNanos' value='33,6']
##teamcity[buildStatisticValue key='AverageDecodeLatencyNanos' value='28,4']

I also tried a BenchmarkDotNet benchmark based on the car example (https://gist.github.com/szehetner/5ed4f7acceedfb8bce092b10f829ba92)

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.18363.959 (1909/November2018Update/19H2)
Intel Core i9-10885H CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.200
  [Host]     : .NET Core 5.0.3 (CoreCLR 5.0.321.7212, CoreFX 5.0.321.7212), X64 RyuJIT
  DefaultJob : .NET Core 5.0.3 (CoreCLR 5.0.321.7212, CoreFX 5.0.321.7212), X64 RyuJIT

before:

Method Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
Encode 189.5 ns 3.77 ns 3.70 ns - - - -
Decode 167.9 ns 1.67 ns 1.39 ns - - - -

after:

Method Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
Encode 158.9 ns 1.93 ns 1.80 ns - - - -
Decode 129.5 ns 0.57 ns 0.48 ns - - - -

Btw, I would suggest to convert the existing benchmark project to BenchmarkDotNet, as this is the de-facto standard for .NET benchmarks. I could create another PR for this if desired.

@billsegall
Copy link
Contributor

Looks good. That's a substantial improvement.

A separate PR for BenchmarkDotNet sounds great.

Thanks!

@mjpt777 mjpt777 merged commit 4237e52 into real-logic:master Mar 31, 2021
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.

3 participants