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 LogEventLevel boxing #1254

Merged
merged 2 commits into from Apr 26, 2019
Merged

Conversation

Pliner
Copy link
Contributor

@Pliner Pliner commented Dec 10, 2018

Hi!

What issue does this PR address?

During the profiling I found that LogEventLevel was boxed during formatting. You can see the evidence of the boxing in the screenshot :

logeventlevel boxing

Does this PR introduce a breaking change?
Nope

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

Other information:

@nblumhardt
Copy link
Member

Nice spotting 👍 - thanks for raising this, and sorry about the slow response over the end-of-year holiday period.

Did you collect any before/after performance numbers? There are some benchmarks in the repository that should give a decent basis for measurement.

@Pliner
Copy link
Contributor Author

Pliner commented Jan 14, 2019

Here are two benchmark results: the baseline(commit hash 5c3a782) and the PR.

As far as I understand cases of the benchmark, the most relevant is OutputTemplateRenderingBenchmark.

net46

BenchmarkDotNet=v0.10.6, OS=Windows 10 Redstone 2 (10.0.15063)
Processor=Intel Core i7-6700 CPU 3.40GHz (Skylake), ProcessorCount=8
Frequency=3328125 Hz, Resolution=300.4695 ns, Timer=TSC
  [Host]     : Clr 4.0.30319.42000, 32bit LegacyJIT-v4.7.3190.0
  DefaultJob : Clr 4.0.30319.42000, 32bit LegacyJIT-v4.7.3190.0

Method Mean Error StdDev Gen 0 Allocated
FormatToOutput 2.280 us 0.0203 us 0.0180 us 0.1144 492 B
Method Mean Error StdDev Gen 0 Allocated
FormatToOutput 1.729 us 0.0077 us 0.0060 us 0.1068 456 B

netcoreapp2.0

BenchmarkDotNet=v0.10.6, OS=Windows 10 Redstone 2 (10.0.15063)
Processor=Intel Core i7-6700 CPU 3.40GHz (Skylake), ProcessorCount=8
Frequency=3328125 Hz, Resolution=300.4695 ns, Timer=TSC
dotnet cli version=2.1.4
  [Host]     : .NET Core 4.6.26614.01, 64bit RyuJIT
  DefaultJob : .NET Core 4.6.26614.01, 64bit RyuJIT

Method Mean Error StdDev Gen 0 Allocated
FormatToOutput 1.315 us 0.0236 us 0.0221 us 0.1163 488 B
Method Mean Error StdDev Gen 0 Allocated
FormatToOutput 1.221 us 0.0116 us 0.0109 us 0.1087 464 B

It seems there is no perfomance degradation 😅

PS Also here are full results.

baseline.zip
pr.zip

@Pliner
Copy link
Contributor Author

Pliner commented Jan 16, 2019

@nblumhardt Is it possible to include the PR to 2.8 release?

@nblumhardt
Copy link
Member

Thanks for the nudge @Pliner - sorry about the slow turnaround on these. Optimisations especially need a close look and I am still a little way from having time to dig into these in detail. Should be low risk to roll them into a 2.8.1, though!

@nblumhardt nblumhardt merged commit ca9664f into serilog:dev Apr 26, 2019
@Pliner Pliner deleted the log_event_level_boxing branch April 26, 2019 14:22
@Pliner
Copy link
Contributor Author

Pliner commented Apr 26, 2019

Thanks!

@nblumhardt nblumhardt mentioned this pull request Oct 12, 2019
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.

None yet

2 participants