Update Json middleware benchmark to use source-gen #1772
Conversation
|
176 MB/sec -> 33 MB/sec with Source gen + new fast path streaming serialization. |
src/Benchmarks/Program.cs
Outdated
| @@ -84,6 +85,11 @@ public static void Main(string[] args) | |||
| } | |||
| #endif | |||
| }) | |||
| .ConfigureHttpJsonOptions(jsonOptions => | |||
| { | |||
| jsonOptions.SerializerOptions.Encoder = null; | |||
There was a problem hiding this comment.
Why do we need this? It seems like a bug.... At least a comment explaining why.
There was a problem hiding this comment.
https://source.dot.net/#System.Text.Json/System/Text/Json/Serialization/JsonSerializerContext.cs,60
The fast path logic requires very specific settings. Didn't add a comment because not sure we want to use this approach in the benchmark.
There was a problem hiding this comment.
Fast-path necessitates compile-time property name encoding, so any custom encoder setting will invalidate it.
There was a problem hiding this comment.
Is this still needed now that we aren't using the JsonOptions anymore?
There was a problem hiding this comment.
Oh wow, we can remove it!
There was a problem hiding this comment.
To confirm we are using "fast path" on both .NET 8 AND 7 with the latest changes, right? Just making sure we are comparing apples-to-apples if we compare them against each other.
There was a problem hiding this comment.
The streaming fast path was added in 8.0, so no, we're not comparing apples-to-apples.
BrennanConroy
force-pushed
the
brecon/json
branch
from
eb2b486
to
a213ea0
Compare
|
|
||
| namespace Benchmarks.Middleware | ||
| { | ||
| public class JsonMiddleware | ||
| { | ||
| private static readonly PathString _path = new PathString(Scenarios.GetPath(s => s.Json)); | ||
| private static readonly UTF8Encoding _encoding = new UTF8Encoding(false); | ||
| private const int _bufferSize = 27; |
There was a problem hiding this comment.
How much performance do we lose if we remove this?
There was a problem hiding this comment.
We lose 20-30k RPS
src/Benchmarks/Program.cs
Outdated
| @@ -84,6 +85,11 @@ public static void Main(string[] args) | |||
| } | |||
| #endif | |||
| }) | |||
| .ConfigureHttpJsonOptions(jsonOptions => | |||
| { | |||
| jsonOptions.SerializerOptions.Encoder = null; | |||
There was a problem hiding this comment.
Is this still needed now that we aren't using the JsonOptions anymore?
179m bytes per second to 3m
Working Set 440MB to 62MB
RPS change unknown right now since the baseline changed between running earlier in the day and now. But running them back-to-back right now gives a ~1.1% increase in RPS