serilog / serilog-sinks-periodicbatching

Infrastructure for Serilog sinks that process events in batches.
Apache License 2.0
71 stars 29 forks source link

VS 2017 / dotnet 1.0.0 tooling #16

Closed skomis-mm closed 7 years ago

skomis-mm commented 7 years ago
nblumhardt commented 7 years ago

All looks good to me. I guess the earlier benchmark run was done on a different machine? (Or maybe without concurrent work going on?) Probably irrelevant, but just noticed that the number seem to have shifted ~20%.

skomis-mm commented 7 years ago

It's the same machine (Processor=Intel(R) Core(TM) i7-4790 CPU 3.60GHz, ProcessorCount=8). Noticeable difference for net452 (not counting BenchmarkDotNet version update) is default platform change: CLR=MS.NET 4.0.30319.42000, Arch=64-bit RELEASE [RyuJIT] - before Clr 4.0.30319.42000, 32bit LegacyJIT-v4.6.1586.0 - after

And for .NET Core the differences are platform version update (1.1 from 1.0) and BenchmarkDotNet version update.

I didn't change code files, just run it as is with default config/job as before. I'll re-run test for net452 with previous config and see if there are any differences.

skomis-mm commented 7 years ago

Ok, there are some differences if I forcibly specify RyuJitX64 job on 64-bit platform that more closely correlates with previous results. So should the tests be forced to use this? Or may be re-run tests for all kind of platforms (x86/x64)?

I don't know why defaults where changed. May be because dotnet test host is now x86. But I would leave that as is without touching tests/config. The final results are not changed the picture as a whole

nblumhardt commented 7 years ago

Sounds like leaving things set at the defaults is fine. It's not really intended to be tracked over time at the moment anyway, since we don't have a dedicated benchmarking machine. Just having a way to measure before/after particular changes seems good enough 👍

Ready to merge this one?

skomis-mm commented 7 years ago

Yes, it's ready now :shipit: