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

Use string create if net6.0 or greater #4183

Merged
merged 17 commits into from Jun 20, 2023
Merged

Conversation

tonyredondo
Copy link
Member

@tonyredondo tonyredondo commented May 25, 2023

Summary of changes

This PR uses the same approach from dotnet/runtime#86685 when generating short strings.

@tonyredondo tonyredondo self-assigned this May 25, 2023
@tonyredondo tonyredondo marked this pull request as ready for review June 12, 2023 21:08
@tonyredondo tonyredondo requested review from a team as code owners June 12, 2023 21:08
@pierotibou
Copy link
Collaborator

QQ, as we don't have benchmarks on master, do you confirm it is actually more performant with this code (it's not just .NET 6 that is better?)

@DataDog DataDog deleted a comment from datadog-ddstaging bot Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from datadog-ddstaging bot Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 14, 2023
@tonyredondo
Copy link
Member Author

QQ, as we don't have benchmarks on master, do you confirm it is actually more performant with this code (it's not just .NET 6 that is better?)

Cannot confirm that, because they are really close and could be just noise. Also I don't see any difference in allocations.

@DataDog DataDog deleted a comment from datadog-ddstaging bot Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@DataDog DataDog deleted a comment from andrewlock Jun 18, 2023
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Jun 18, 2023

Datadog Report

Branch report: tony/use-string-create
Commit report: 66f17ab

dd-trace-dotnet: 0 Failed, 0 New Flaky, 294122 Passed, 949 Skipped, 34m 31.72s Wall Time

@andrewlock
Copy link
Member

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4183) - mean (3,021ms)  : 2924, 3118
     .   : milestone, 3021,
    master - mean (3,008ms)  : 2920, 3096
     .   : milestone, 3008,

    section CallTarget+Inlining+NGEN
    This PR (4183) - mean (3,799ms)  : 3709, 3888
     .   : milestone, 3799,
    master - mean (3,814ms)  : 3726, 3901
     .   : milestone, 3814,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4183) - mean (3,117ms)  : 3009, 3225
     .   : milestone, 3117,
    master - mean (3,118ms)  : 3014, 3221
     .   : milestone, 3118,

    section CallTarget+Inlining+NGEN
    This PR (4183) - mean (3,638ms)  : 3557, 3719
     .   : milestone, 3638,
    master - mean (3,624ms)  : 3570, 3678
     .   : milestone, 3624,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4183) - mean (3,110ms)  : 3009, 3210
     .   : milestone, 3110,
    master - mean (3,095ms)  : 3001, 3189
     .   : milestone, 3095,

    section CallTarget+Inlining+NGEN
    This PR (4183) - mean (3,587ms)  : 3530, 3645
     .   : milestone, 3587,
    master - mean (3,629ms)  : 3521, 3738
     .   : milestone, 3629,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4183) - mean (189ms)  : 185, 193
     .   : milestone, 189,
    master - mean (190ms)  : 185, 194
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (4183) - mean (1,068ms)  : 1013, 1123
     .   : milestone, 1068,
    master - mean (1,070ms)  : 997, 1143
     .   : milestone, 1070,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4183) - mean (369ms)  : 363, 374
     .   : milestone, 369,
    master - mean (372ms)  : 365, 379
     .   : milestone, 372,

    section CallTarget+Inlining+NGEN
    This PR (4183) - mean (1,157ms)  : 1131, 1184
     .   : milestone, 1157,
    master - mean (1,157ms)  : 1126, 1189
     .   : milestone, 1157,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4183) - mean (358ms)  : 352, 365
     .   : milestone, 358,
    master - mean (360ms)  : 352, 368
     .   : milestone, 360,

    section CallTarget+Inlining+NGEN
    This PR (4183) - mean (1,118ms)  : 1085, 1151
     .   : milestone, 1118,
    master - mean (1,119ms)  : 1090, 1148
     .   : milestone, 1119,

@andrewlock
Copy link
Member

Benchmarks Report 🐌

Benchmarks for #4183 compared to master:

  • 2 benchmarks are faster, with geometric mean 1.164
  • 1 benchmarks have fewer allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces netcoreapp3.1 624μs 79.8ns 288ns 0 0 0 2.62 KB
master WriteAndFlushEnrichedTraces net472 805μs 278ns 1μs 0.403 0 0 3.22 KB
#4183 WriteAndFlushEnrichedTraces net6.0 473μs 338ns 1.31μs 0 0 0 2.62 KB
#4183 WriteAndFlushEnrichedTraces netcoreapp3.1 638μs 134ns 518ns 0 0 0 2.63 KB
#4183 WriteAndFlushEnrichedTraces net472 804μs 569ns 2.13μs 0.408 0 0 3.22 KB
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody netcoreapp3.1 40.9μs 71.3ns 276ns 0 0 0 1.63 KB
master AllCycleSimpleBody net472 42.4μs 42.1ns 163ns 0.251 0 0 1.69 KB
master AllCycleMoreComplexBody netcoreapp3.1 231μs 383ns 1.48μs 0.115 0 0 9.12 KB
master AllCycleMoreComplexBody net472 238μs 900ns 4.03μs 1.42 0 0 9.28 KB
master ObjectExtractorSimpleBody netcoreapp3.1 183ns 0.299ns 1.16ns 0.00369 0 0 272 B
master ObjectExtractorSimpleBody net472 144ns 0.186ns 0.72ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody netcoreapp3.1 4.05μs 2.07ns 7.46ns 0.0507 0 0 3.78 KB
master ObjectExtractorMoreComplexBody net472 4.14μs 2.06ns 7.41ns 0.617 0.00621 0 3.89 KB
#4183 AllCycleSimpleBody net6.0 39.2μs 19.9ns 77.2ns 0.019 0 0 1.65 KB
#4183 AllCycleSimpleBody netcoreapp3.1 40.6μs 103ns 385ns 0.0203 0 0 1.63 KB
#4183 AllCycleSimpleBody net472 41μs 162ns 627ns 0.264 0 0 1.69 KB
#4183 AllCycleMoreComplexBody net6.0 219μs 50.9ns 197ns 0.109 0 0 9.22 KB
#4183 AllCycleMoreComplexBody netcoreapp3.1 228μs 361ns 1.4μs 0.113 0 0 9.12 KB
#4183 AllCycleMoreComplexBody net472 235μs 36.5ns 132ns 1.41 0 0 9.28 KB
#4183 ObjectExtractorSimpleBody net6.0 119ns 0.0285ns 0.107ns 0.00391 0 0 280 B
#4183 ObjectExtractorSimpleBody netcoreapp3.1 178ns 0.0725ns 0.271ns 0.00377 0 0 272 B
#4183 ObjectExtractorSimpleBody net472 144ns 0.153ns 0.593ns 0.0446 0 0 281 B
#4183 ObjectExtractorMoreComplexBody net6.0 3.02μs 0.772ns 2.89ns 0.0544 0 0 3.88 KB
#4183 ObjectExtractorMoreComplexBody netcoreapp3.1 3.95μs 1.38ns 4.98ns 0.0513 0 0 3.78 KB
#4183 ObjectExtractorMoreComplexBody net472 4.09μs 1.81ns 6.52ns 0.618 0.00614 0 3.89 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest netcoreapp3.1 188μs 150ns 579ns 0.187 0 0 20.08 KB
master SendRequest net472 0.000965ns 0.000367ns 0.00137ns 0 0 0 0 b
#4183 SendRequest net6.0 165μs 80.7ns 313ns 0.246 0 0 18.02 KB
#4183 SendRequest netcoreapp3.1 186μs 286ns 1.11μs 0.185 0 0 20.08 KB
#4183 SendRequest net472 0.000444ns 0.000225ns 0.000842ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #4183

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 41.79 KB 41.57 KB -220 B -0.53%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces netcoreapp3.1 632μs 348ns 1.26μs 0.314 0 0 41.79 KB
master WriteAndFlushEnrichedTraces net472 798μs 1.59μs 5.94μs 8.2 2.34 0.391 53.23 KB
#4183 WriteAndFlushEnrichedTraces net6.0 500μs 1.59μs 6.15μs 0.496 0 0 41.83 KB
#4183 WriteAndFlushEnrichedTraces netcoreapp3.1 633μs 1.77μs 6.85μs 0.309 0 0 41.57 KB
#4183 WriteAndFlushEnrichedTraces net472 781μs 3.6μs 13.9μs 8.31 2.37 0.396 53.23 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery netcoreapp3.1 1.08μs 0.298ns 1.07ns 0.00974 0 0 720 B
master ExecuteNonQuery net472 1.31μs 1.21ns 4.68ns 0.108 0 0 682 B
#4183 ExecuteNonQuery net6.0 874ns 0.257ns 0.994ns 0.0102 0 0 720 B
#4183 ExecuteNonQuery netcoreapp3.1 1.06μs 0.182ns 0.631ns 0.00968 0 0 720 B
#4183 ExecuteNonQuery net472 1.42μs 0.46ns 1.59ns 0.108 0.000708 0 682 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch netcoreapp3.1 1.28μs 0.579ns 2.09ns 0.0127 0 0 944 B
master CallElasticsearch net472 1.92μs 0.484ns 1.87ns 0.151 0.000963 0 955 B
master CallElasticsearchAsync netcoreapp3.1 1.31μs 0.676ns 2.53ns 0.013 0 0 992 B
master CallElasticsearchAsync net472 2.16μs 0.896ns 3.47ns 0.16 0.00108 0 1.01 KB
#4183 CallElasticsearch net6.0 850ns 0.231ns 0.863ns 0.0131 0 0 944 B
#4183 CallElasticsearch netcoreapp3.1 1.22μs 0.276ns 0.996ns 0.0128 0 0 944 B
#4183 CallElasticsearch net472 2.08μs 0.493ns 1.84ns 0.152 0.00104 0 955 B
#4183 CallElasticsearchAsync net6.0 887ns 0.264ns 0.989ns 0.0129 0 0 920 B
#4183 CallElasticsearchAsync netcoreapp3.1 1.25μs 0.483ns 1.67ns 0.0134 0 0 992 B
#4183 CallElasticsearchAsync net472 2.08μs 0.576ns 2.23ns 0.16 0.00104 0 1.01 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync netcoreapp3.1 1.32μs 0.365ns 1.37ns 0.012 0 0 912 B
master ExecuteAsync net472 1.49μs 0.575ns 2.07ns 0.138 0.000744 0 875 B
#4183 ExecuteAsync net6.0 1.06μs 0.53ns 1.98ns 0.0128 0 0 912 B
#4183 ExecuteAsync netcoreapp3.1 1.3μs 1.61ns 6.22ns 0.0122 0 0 912 B
#4183 ExecuteAsync net472 1.54μs 0.518ns 1.94ns 0.139 0.000772 0 875 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync netcoreapp3.1 4.3μs 2.08ns 8.04ns 0.0323 0 0 2.42 KB
master SendAsync net472 6.83μs 1.73ns 6.69ns 0.474 0 0 2.99 KB
#4183 SendAsync net6.0 3.34μs 0.595ns 2.14ns 0.0264 0 0 1.89 KB
#4183 SendAsync netcoreapp3.1 4.13μs 1.13ns 4.36ns 0.0331 0 0 2.42 KB
#4183 SendAsync net472 6.79μs 2.42ns 9.38ns 0.474 0 0 2.99 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog netcoreapp3.1 1.8μs 1.02ns 3.93ns 0.0226 0 0 1.62 KB
master EnrichedLog net472 2.22μs 0.836ns 3.24ns 0.244 0 0 1.54 KB
#4183 EnrichedLog net6.0 1.27μs 0.457ns 1.71ns 0.0229 0 0 1.62 KB
#4183 EnrichedLog netcoreapp3.1 1.82μs 2.51ns 9.73ns 0.0217 0 0 1.62 KB
#4183 EnrichedLog net472 2.22μs 1.6ns 6.21ns 0.244 0 0 1.54 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog netcoreapp3.1 118μs 121ns 468ns 0.0584 0 0 4.21 KB
master EnrichedLog net472 148μs 90.4ns 350ns 0.665 0.222 0 4.38 KB
#4183 EnrichedLog net6.0 109μs 121ns 471ns 0.0544 0 0 4.21 KB
#4183 EnrichedLog netcoreapp3.1 117μs 256ns 957ns 0.0575 0 0 4.21 KB
#4183 EnrichedLog net472 147μs 153ns 591ns 0.651 0.217 0 4.38 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog netcoreapp3.1 3.69μs 1.07ns 3.87ns 0.05 0 0 3.69 KB
master EnrichedLog net472 4.79μs 11.5ns 44.5ns 0.526 0.00237 0 3.31 KB
#4183 EnrichedLog net6.0 3.08μs 0.85ns 3.07ns 0.0522 0 0 3.71 KB
#4183 EnrichedLog netcoreapp3.1 3.67μs 1.29ns 4.65ns 0.0514 0 0 3.69 KB
#4183 EnrichedLog net472 4.7μs 1.15ns 4.31ns 0.524 0.00235 0 3.31 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive netcoreapp3.1 1.38μs 0.288ns 1.04ns 0.0131 0 0 1 KB
master SendReceive net472 1.68μs 1.59ns 6.17ns 0.159 0 0 1 KB
#4183 SendReceive net6.0 1.07μs 0.439ns 1.64ns 0.0138 0 0 1 KB
#4183 SendReceive netcoreapp3.1 1.44μs 0.294ns 1.1ns 0.0137 0 0 1 KB
#4183 SendReceive net472 1.73μs 0.719ns 2.49ns 0.159 0 0 1 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog netcoreapp3.1 3.58μs 1.6ns 6ns 0.0198 0 0 1.58 KB
master EnrichedLog net472 4.01μs 4.76ns 18.4ns 0.31 0 0 1.96 KB
#4183 EnrichedLog net6.0 2.42μs 0.706ns 2.55ns 0.0212 0 0 1.53 KB
#4183 EnrichedLog netcoreapp3.1 3.42μs 0.791ns 2.85ns 0.0207 0 0 1.58 KB
#4183 EnrichedLog net472 3.95μs 2.08ns 8.06ns 0.31 0 0 1.96 KB
Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #4183

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net472 1.143 847.84 741.58

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan netcoreapp3.1 475ns 0.639ns 2.47ns 0.00733 0 0 536 B
master StartFinishSpan net472 635ns 0.233ns 0.871ns 0.0854 0 0 538 B
master StartFinishScope netcoreapp3.1 646ns 0.257ns 0.962ns 0.00875 0 0 656 B
master StartFinishScope net472 848ns 0.296ns 1.07ns 0.0982 0 0 618 B
#4183 StartFinishSpan net6.0 357ns 0.101ns 0.377ns 0.00762 0 0 536 B
#4183 StartFinishSpan netcoreapp3.1 505ns 0.307ns 1.19ns 0.00725 0 0 536 B
#4183 StartFinishSpan net472 630ns 0.324ns 1.25ns 0.0852 0 0 538 B
#4183 StartFinishScope net6.0 471ns 0.121ns 0.453ns 0.00912 0 0 656 B
#4183 StartFinishScope netcoreapp3.1 703ns 1.28ns 4.8ns 0.00898 0 0 656 B
#4183 StartFinishScope net472 741ns 0.235ns 0.911ns 0.098 0 0 618 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #4183

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑netcoreapp3.1 1.184 768.12 648.60

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin netcoreapp3.1 768ns 0.305ns 1.18ns 0.00886 0 0 656 B
master RunOnMethodBegin net472 1.01μs 0.191ns 0.74ns 0.0978 0 0 618 B
#4183 RunOnMethodBegin net6.0 578ns 0.26ns 1.01ns 0.00921 0 0 656 B
#4183 RunOnMethodBegin netcoreapp3.1 648ns 0.228ns 0.789ns 0.00878 0 0 656 B
#4183 RunOnMethodBegin net472 928ns 0.237ns 0.886ns 0.0978 0 0 618 B

@andrewlock
Copy link
Member

Throughput/Crank Report:zap:

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4183) (11.282M)   : 0, 11282157
    master (11.444M)   : 0, 11443637
    benchmarks/2.31.0 (11.331M)   : 0, 11330560
    benchmarks/2.9.0 (11.301M)   : 0, 11301309

    section Automatic
    This PR (4183) (8.075M)   : 0, 8074884
    master (8.028M)   : 0, 8027516
    benchmarks/2.31.0 (8.087M)   : 0, 8086789
    benchmarks/2.9.0 (8.193M)   : 0, 8192771

    section Trace stats
    master (8.033M)   : 0, 8032955
    benchmarks/2.31.0 (8.031M)   : 0, 8031178

    section Manual
    This PR (4183) (10.073M)   : 0, 10073433
    master (10.296M)   : 0, 10296400
    benchmarks/2.31.0 (10.125M)   : 0, 10125369

    section Manual + Automatic
    This PR (4183) (7.607M)   : 0, 7606635
    master (7.775M)   : 0, 7775481
    benchmarks/2.31.0 (7.775M)   : 0, 7775145

    section Version Conflict
    master (6.991M)   : 0, 6991290
    benchmarks/2.31.0 (6.977M)   : 0, 6977309

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4183) (9.562M)   : 0, 9562043
    master (9.492M)   : 0, 9491825
    benchmarks/2.31.0 (9.462M)   : 0, 9461591
    benchmarks/2.9.0 (9.457M)   : 0, 9457357

    section Automatic
    This PR (4183) (6.673M)   : 0, 6672610
    master (6.984M)   : 0, 6983612
    benchmarks/2.31.0 (6.609M)   : 0, 6608731

    section Trace stats
    master (6.809M)   : 0, 6808981
    benchmarks/2.31.0 (6.751M)   : 0, 6750564

    section Manual
    This PR (4183) (8.494M)   : 0, 8493642
    master (8.392M)   : 0, 8392081
    benchmarks/2.31.0 (8.632M)   : 0, 8631992

    section Manual + Automatic
    This PR (4183) (6.573M)   : 0, 6572611
    master (6.690M)   : 0, 6689904
    benchmarks/2.31.0 (6.536M)   : 0, 6535693

    section Version Conflict
    master (5.873M)   : 0, 5872703
    benchmarks/2.31.0 (6.058M)   : 0, 6057526

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4183) (9.561M)   : 0, 9561373
    master (10.487M)   : 0, 10486575
    benchmarks/2.9.0 (9.422M)   : 0, 9421865

    section Automatic
    This PR (4183) (6.923M)   : crit ,0, 6923189
    master (7.547M)   : 0, 7546661
    benchmarks/2.9.0 (7.462M)   : 0, 7461655

    section Trace stats
    master (7.488M)   : 0, 7488208

    section Manual
    This PR (4183) (8.639M)   : crit ,0, 8638501
    master (9.435M)   : 0, 9435451

    section Manual + Automatic
    This PR (4183) (6.699M)   : crit ,0, 6698663
    master (7.296M)   : 0, 7295579

    section Version Conflict
    master (6.688M)   : 0, 6688167

gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4183) (7.263M)   : 0, 7263333
    master (7.248M)   : 0, 7248222
    benchmarks/2.31.0 (7.243M)   : 0, 7243146
    benchmarks/2.9.0 (7.622M)   : 0, 7622034

    section No attack
    This PR (4183) (2.094M)   : 0, 2094241
    master (2.051M)   : 0, 2051432
    benchmarks/2.31.0 (2.364M)   : 0, 2363993
    benchmarks/2.9.0 (3.164M)   : 0, 3163666

    section Attack
    This PR (4183) (1.799M)   : 0, 1798820
    master (1.804M)   : 0, 1804389
    benchmarks/2.31.0 (2.004M)   : 0, 2004139
    benchmarks/2.9.0 (2.518M)   : 0, 2517779

    section Blocking
    This PR (4183) (3.427M)   : 0, 3426620
    master (3.442M)   : 0, 3441577
    benchmarks/2.31.0 (3.967M)   : 0, 3967236

@tonyredondo
Copy link
Member Author

QQ, as we don't have benchmarks on master, do you confirm it is actually more performant with this code (it's not just .NET 6 that is better?)

Cannot confirm that, because they are really close and could be just noise. Also I don't see any difference in allocations.

Actually benchmarks are showing some improvements.

Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take your word for it on the benchmarks. This string.Create overload is awesome.

@@ -90,7 +90,11 @@ public static void GetMessageHeaders(EventArgs? eventArgs, out IServiceRemotingR

public static string GetSpanName(string spanKind)
{
#if NET6_0_OR_GREATER
return string.Create(null, stackalloc char[128], $"{SpanNamePrefix}.{spanKind}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically there's a potential issue here if spanKind.Length > 128 - SpanNamePrefix.Length. It's very unlikely, and this is internal and we control the spanKind types in this callpath. But it still gives me slight eye twitch! 😄

@tonyredondo tonyredondo merged commit 89ce11d into master Jun 20, 2023
86 checks passed
@tonyredondo tonyredondo deleted the tony/use-string-create branch June 20, 2023 13:43
@github-actions github-actions bot added this to the vNext milestone Jun 20, 2023
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

4 participants