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

Adds .NET distributed tracing instrumentation & metrics. #1200

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fdcastel
Copy link
Contributor

Initial work for #1196.

@fdcastel fdcastel changed the title Add telemetry trace Adds .NET distributed tracing instrumentation. Oct 25, 2024
@fdcastel fdcastel changed the title Adds .NET distributed tracing instrumentation. Adds .NET distributed tracing instrumentation & metrics. Oct 26, 2024
@fdcastel
Copy link
Contributor Author

Included the final portion of #1196 (metrics) here, as it requires the same infrastructure as traces.

@willibrandon
Copy link
Contributor

Here is before and after comparison of Perf benchmark results on my machine.

BenchmarkDotNet=v0.13.2, OS=Windows 11 (10.0.26120.2130)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=9.0.100-rc.2.24474.11
  [Host]  : .NET 8.0.10 (8.0.1024.46610), X64 RyuJIT AVX2
  NuGet   : .NET 6.0.35 (6.0.3524.45918), X64 RyuJIT AVX2
  Project : .NET 6.0.35 (6.0.3524.45918), X64 RyuJIT AVX2

Jit=RyuJit  Platform=X64  Toolchain=.NET 6.0  
WarmupCount=3  

Perf.CommandBenchmark

FirebirdSQL:master (Old) <-> fdcastel:add-telemetry-trace (New)

Diff Method Mean Error Allocated
Old Execute 40,075.4 μs 1,450.87 μs 313.71 KB
New 34,322.0 μs (-14%) 658.70 μs 313.11 KB (0%)
Old Execute 34,465.9 μs 680.65 μs 311.3 KB
New 34,248.1 μs (-1%) 384.64 μs 315.91 KB (+1%)
Old Fetch 640.1 μs 11.02 μs 56.88 KB
New 648.4 μs (+1%) 12.19 μs 56.88 KB (0%)
Old Fetch 653.2 μs 10.90 μs 51.43 KB
New 653.1 μs (0%) 9.41 μs 56.06 KB (+9%)
Old Execute 33,690.3 μs 648.64 μs 319.86 KB
New 33,559.9 μs (0%) 664.54 μs 319.85 KB (0%)
Old Execute 32,943.5 μs 235.67 μs 317.28 KB
New 34,278.5 μs (+4%) 683.34 μs 322.01 KB (+1%)
Old Fetch 642.2 μs 3.47 μs 60.75 KB
New 647.3 μs (+1%) 12.89 μs 60.75 KB (0%)
Old Fetch 664.2 μs 7.49 μs 55.31 KB
New 658.7 μs (-1%) 8.82 μs 59.92 KB (+8%)

@fdcastel
Copy link
Contributor Author

fdcastel commented Oct 26, 2024

Thanks @willibrandon. This surely can be improved.

Could you kindly share the steps/configuration you used for the above benchmark results?

Mine here produces a comparison between Release (built from local sources) and ReleaseNuget (NuGet package for FirebirdClient 8.5.4).

@fdcastel fdcastel mentioned this pull request Oct 26, 2024
@fdcastel
Copy link
Contributor Author

@willibrandon I pushed a new PR in #1203 (it is based on this PR).

Could you please clone it and run ./run-benchmark.ps1 in your system?

The script will compare the current project (built from sources with .NET8) with the latest NuGet release (10.3.1).

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22621.4317/22H2/2022Update/SunValley2)
13th Gen Intel Core i7-13700T, 1 CPU, 24 logical and 16 physical cores
.NET SDK 9.0.100-rc.2.24474.11
  [Host]  : .NET 8.0.10 (8.0.1024.46610), X64 RyuJIT AVX2
  Core80  : .NET 8.0.10 (8.0.1024.46610), X64 RyuJIT AVX2
  NuGet80 : .NET 8.0.10 (8.0.1024.46610), X64 RyuJIT AVX2

Jit=RyuJit  Platform=X64  Toolchain=.NET 8.0
WarmupCount=3

| Method  | Job     | BuildConfiguration | DataType             | Count | Mean        | Error     | StdDev    | Ratio | RatioSD | Gen0   | Allocated | Alloc Ratio |
|-------- |-------- |------------------- |--------------------- |------ |------------:|----------:|----------:|------:|--------:|-------:|----------:|------------:|
| Execute | Core80  | Release            | BIGINT               | 100   | 16,219.7 us | 140.65 us | 131.57 us |  1.00 |    0.01 |      - | 306.16 KB |        1.01 |
| Execute | NuGet80 | ReleaseNuGet       | BIGINT               | 100   | 16,283.9 us |  29.35 us |  26.01 us |  1.00 |    0.00 |      - | 301.65 KB |        1.00 |
|         |         |                    |                      |       |             |           |           |       |         |        |           |             |
| Fetch   | Core80  | Release            | BIGINT               | 100   |    372.1 us |   2.91 us |   2.43 us |  1.00 |    0.01 | 2.9297 |  55.66 KB |        1.09 |
| Fetch   | NuGet80 | ReleaseNuGet       | BIGINT               | 100   |    371.3 us |   2.47 us |   2.19 us |  1.00 |    0.01 | 2.9297 |  51.15 KB |        1.00 |
|         |         |                    |                      |       |             |           |           |       |         |        |           |             |
| Execute | Core80  | Release            | VARCH(...) UTF8 [30] | 100   | 18,021.3 us | 352.58 us | 482.62 us |  1.00 |    0.03 |      - |  310.1 KB |        1.01 |
| Execute | NuGet80 | ReleaseNuGet       | VARCH(...) UTF8 [30] | 100   | 18,071.4 us | 352.48 us | 377.14 us |  1.00 |    0.03 |      - | 305.64 KB |        1.00 |
|         |         |                    |                      |       |             |           |           |       |         |        |           |             |
| Fetch   | Core80  | Release            | VARCH(...) UTF8 [30] | 100   |    416.7 us |   6.50 us |   5.43 us |  1.01 |    0.03 | 2.9297 |  59.54 KB |        1.08 |
| Fetch   | NuGet80 | ReleaseNuGet       | VARCH(...) UTF8 [30] | 100   |    413.6 us |   8.09 us |  10.52 us |  1.00 |    0.03 | 2.9297 |  55.02 KB |        1.00 |

// * Hints *
Outliers
  CommandBenchmark.Execute: NuGet80 -> 1 outlier  was  removed (17.09 ms)
  CommandBenchmark.Fetch: Core80    -> 2 outliers were removed (383.60 us, 403.48 us)
  CommandBenchmark.Fetch: NuGet80   -> 1 outlier  was  removed (389.54 us)
  CommandBenchmark.Execute: Core80  -> 1 outlier  was  removed, 3 outliers were detected (16.46 ms, 16.87 ms, 19.28 ms)
  CommandBenchmark.Fetch: Core80    -> 2 outliers were removed (442.98 us, 447.90 us)

// * Legends *
  DataType    : Value of the 'DataType' parameter
  Count       : Value of the 'Count' parameter
  Mean        : Arithmetic mean of all measurements
  Error       : Half of 99.9% confidence interval
  StdDev      : Standard deviation of all measurements
  Ratio       : Mean of the ratio distribution ([Current]/[Baseline])
  RatioSD     : Standard deviation of the ratio distribution ([Current]/[Baseline])
  Gen0        : GC Generation 0 collects per 1000 operations
  Allocated   : Allocated memory per single operation (managed only, inclusive, 1KB = 1024B)
  Alloc Ratio : Allocated memory ratio distribution ([Current]/[Baseline])
  1 us        : 1 Microsecond (0.000001 sec)

// * Diagnostic Output - MemoryDiagnoser *


// ***** BenchmarkRunner: End *****
Run time: 00:01:41 (101.22 sec), executed benchmarks: 8

Global total time: 00:01:51 (111.73 sec), executed benchmarks: 8

@willibrandon
Copy link
Contributor

@fdcastel - Interesting, and thanks! Taking a look now.
I had the same thought the Perf project toolchain was out of date.

@willibrandon
Copy link
Contributor

Okay, I see how the performance project works now with the conditional project/package reference, and with the latest Nuget release being the baseline. That is way better approach than the way I was attempting to compare the the results historically.

@fdcastel - Thank you very much for showing me how the performance project works. I just want try and confirm we haven't regressed in someway with this change.

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