Update JSON platform benchmarks to use JSON source generator by layomia · Pull Request #1683 · aspnet/Benchmarks

layomia · 2021-06-20T01:49:28Z

Profile	Scenario	RPS (Before)	RPS (After)	RPS % increase	Max alloc rate (Before) (b/sec)	Max alloc rate (After) (b/sec)	Max alloc rate % decrease	Latency (Before) (ms)	Latency (After) (ms)	Latency % decrease	Start time + first request (Before) (ms)	Start time (After)	% Start time decrease
aspnet-citrine-lin	json	1,232,440	1,265,521	2.684187466	41,672,184	12,429,712	70.17264082	0.57	0.51	10.52631579	246	262	-6.504065041
	multiple_queries	23,452	23,434	-0.07675251578	752,562,891	727,026,011	3.393321715	10.92	11.01	-0.8241758242	1725	1735	-0.5797101449
	single_query	397,556	402,294	1.191781787	1,084,997,824	1,098,367,805	-1.232258785	0.66	0.66	0	1940	1831	5.618556701
	updates	18,276	18,478	1.105274677	1,072,226,685	1,067,972,931	0.3967215198	14	13.86	1	1955	1961	-0.3069053708

We see a small increase in RPS and a big drop in max alloc rate for the json benchmark. There's also a small increase in RPS for the single_query benchmark. Otherwise, the rest of the benchmarks don't change substantially.

Benchmarks were run in 6 iterations; excluding the worst two and the best result; then taking the mean.
Benchmarks had 30s warmup.
Benchmarks run on aspnet-citrine-lin profile.

crank --config platform.benchmarks.yml --scenario json --profile aspnet-citrine-lin --variable warmup=30 --iterations 6 --exclude 2:1 --exclude-order "load:wrk/rps/mean;http/rps/mean" --application.options.collectCounters true

FYI @sebastienros @adamsitnik @eerhardt @steveharter @ericstj @stephentoub @jkotas @davidfowl

src/BenchmarksApps/Kestrel/PlatformBenchmarks/BenchmarkApplication.Json.cs

adamsitnik · 2021-06-21T08:31:55Z

@layomia I was looking forward to find out what is the difference for Caching benchmark, where so far 40% of time was spent for JSON serialization. So I run the benchmarks against your fork:

load	net5.0	net6.0	this PR
CPU Usage (%)	20	21	30
Cores usage (%)	556	597	842
Working Set (MB)	37	38	37
Private Memory (MB)	358	358	358
Start Time (ms)	0	0	0
First Request (ms)	66	67	62
Requests/sec	243,000	260,928	364,224
Requests	3,669,151	3,939,804	5,499,468
Mean latency (ms)	1.22	1.09	0.90
Max latency (ms)	57.32	47.40	44.63
Bad responses	0	0	0
Socket errors	0	0	0
Read throughput (MB/s)	764.86	821.29	1,146.88
Latency 50th (ms)	1.03	0.96	0.67
Latency 75th (ms)	1.14	1.06	0.76
Latency 90th (ms)	1.28	1.19	0.87
Latency 99th (ms)	9.57	6.34	9.40

That gives us a 100k RPS GAIN which is almost +30%.

cc @jeffhandley @maryamariyan

adamsitnik

This is simply most impressive. Great work @layomia !

layomia · 2021-06-21T15:24:41Z

Thanks @adamsitnik! FWIW I set out wanting to measure the caching benchmark diff, but somehow thought it was the "multiple_queries" scenario. Happy the RPS improved, as we hoped!

src/BenchmarksApps/Kestrel/PlatformBenchmarks/PlatformBenchmarks.csproj

sebastienros · 2021-07-19T10:33:12Z

Not seeing the whole changeset, would this break on 3.1 or 5.0? Last time I checked the PR it would require different versions of the code.

layomia · 2021-07-19T16:18:29Z

@sebastienros the 3.1 and 5.0 implementations are unchanged in this PR, meaning that they don't use the source generator and things shouldn't break. This is based on ifdef'ing the new code based on NET6_0_OR_GREATER. FWIW the generator works well with these TFMs, but we're deciding not to use it for those cases.

halter73

LGTM

halter73 · 2021-07-20T02:54:12Z

Thanks @layomia

…1683)" This reverts commit bf3490f.

…1683)" (#1688) This reverts commit bf3490f.

…nerator (#1683)" (#1688)" This reverts commit 66360ad.

…nerator (#1683)" (#1688)" (#1698) This reverts commit 66360ad.

layomia commented Jun 20, 2021

View reviewed changes

src/BenchmarksApps/Kestrel/PlatformBenchmarks/BenchmarkApplication.Json.cs Outdated Show resolved Hide resolved

layomia force-pushed the JsonSourceGenerator_Serializer branch from d53e6de to 79b1285 Compare June 20, 2021 01:55

adamsitnik approved these changes Jun 21, 2021

View reviewed changes

adamsitnik mentioned this pull request Jun 21, 2021

Regression in Caching TechEmpower benchmark dotnet/runtime#52640

Closed

layomia mentioned this pull request Jun 21, 2021

Add code to flush JSON writer after root-level fast-path serialization dotnet/runtime#54502

Merged

sebastienros reviewed Jun 21, 2021

View reviewed changes

src/BenchmarksApps/Kestrel/PlatformBenchmarks/PlatformBenchmarks.csproj Outdated Show resolved Hide resolved

layomia force-pushed the JsonSourceGenerator_Serializer branch from f1cfdff to b637b37 Compare June 26, 2021 21:24

layomia commented Jul 9, 2021

View reviewed changes

src/BenchmarksApps/Kestrel/PlatformBenchmarks/PlatformBenchmarks.csproj Outdated Show resolved Hide resolved

layomia and others added 4 commits July 14, 2021 20:58

Update JSON platform benchmarks to use JSON source generator

437ca83

Address review feedback and use updated API

4d201e0

Use non-pinned/latest STJ version

bf5dd90

Remove STJ package ref

cb783e0

layomia force-pushed the JsonSourceGenerator_Serializer branch from 4301e47 to cb783e0 Compare July 15, 2021 04:10

halter73 approved these changes Jul 20, 2021

View reviewed changes

halter73 merged commit bf3490f into aspnet:main Jul 20, 2021

layomia mentioned this pull request Jul 21, 2021

Compile-time source generation for System.Text.Json dotnet/runtime#45448

Closed

23 tasks

davidfowl added a commit that referenced this pull request Jul 22, 2021

Revert "Update JSON platform benchmarks to use JSON source generator (#…

7be7e7b

…1683)" This reverts commit bf3490f.

davidfowl mentioned this pull request Jul 22, 2021

Revert "Update JSON platform benchmarks to use JSON source generator … #1688

Merged

davidfowl added a commit that referenced this pull request Jul 22, 2021

Revert "Update JSON platform benchmarks to use JSON source generator (#…

66360ad

…1683)" (#1688) This reverts commit bf3490f.

halter73 mentioned this pull request Jul 26, 2021

Re-update JSON platform benchmarks to use JSON source generator #1689

Closed

sebastienros added a commit that referenced this pull request Sep 7, 2021

Revert "Revert "Update JSON platform benchmarks to use JSON source ge…

fdb5ede

…nerator (#1683)" (#1688)" This reverts commit 66360ad.

sebastienros added a commit that referenced this pull request Sep 17, 2021

Revert "Revert "Update JSON platform benchmarks to use JSON source ge…

d9244aa

…nerator (#1683)" (#1688)" (#1698) This reverts commit 66360ad.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update JSON platform benchmarks to use JSON source generator#1683

Update JSON platform benchmarks to use JSON source generator#1683
halter73 merged 4 commits intoaspnet:mainfrom
layomia:JsonSourceGenerator_Serializer

layomia commented Jun 20, 2021

Uh oh!

Uh oh!

adamsitnik commented Jun 21, 2021

Uh oh!

adamsitnik left a comment

Uh oh!

layomia commented Jun 21, 2021

Uh oh!

Uh oh!

Uh oh!

sebastienros commented Jul 19, 2021

Uh oh!

layomia commented Jul 19, 2021

Uh oh!

halter73 left a comment

Uh oh!

halter73 commented Jul 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

layomia commented Jun 20, 2021

Uh oh!

Uh oh!

adamsitnik commented Jun 21, 2021

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

layomia commented Jun 21, 2021

Uh oh!

Uh oh!

Uh oh!

sebastienros commented Jul 19, 2021

Uh oh!

layomia commented Jul 19, 2021

Uh oh!

halter73 left a comment

Choose a reason for hiding this comment

Uh oh!

halter73 commented Jul 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants