SwiftQUIC: PERF: Reduce CPU by 63% parsing QUIC header

[agnosticdev]

Parsing QUIC headers with Deserializer.deserialize uses a lot more CPU than expected for what should be a very fast operation. Parsing the firstOctet and short headers are by far the common cases when parsing QUIC packets, and the paths that should be the most performant. This change is to provide a fast-path option for these operations and cut the CPU usage here by 63% when parsing QUIC headers in the QUICTransfer benchmark.

Savings of about 260 megacycles.

Current top of tree:

417.04 M 52.8%  -                                             PacketParser.parseHeader(frame:dcidLength:)   
417.04 M 52.8%  30.48 M                                        specialized PacketParser.parseHeader(frame:dcidLength:)  
153.60 M 19.5%  12.00 M                                         PacketParser.parseShortHeader(frame:firstOctet:dcidLength:originalLength:)  
107.87 M 13.7%  5.00 M                                           static Deserializer<>.deserialize<>(_:claim:_:)    
41.72 M  5.3%  5.00 M                                            Frame.claim(fromStart:fromEnd:adjustSingleIPAggregate:)   
27.11 M  3.4%  27.11 M                                            0x192be0ce8 (libsystem_pthread.dylib +0x1ce9) <56F332B8-6B78-3DB7-A58C-F3F4EC03D383> 
5.00 M   0.6%  5.00 M                                             type metadata accessor for Logger    
2.60 M   0.3%  2.60 M                                             Frame.startOffset.setter 
2.00 M   0.3%  2.00 M                                             DYLD-STUB$$type metadata accessor for Logger 
38.15 M  4.8%  -                                                 static Deserializer<>.deserialize<>(_:_:) 
23.00 M  2.9%  22.00 M                                           Frame.bytes.getter    
26.73 M  3.4%  -                                                QUICConnectionID.init(storage:size:)   
26.73 M  3.4%  9.13 M                                            specialized QUICConnectionID.init(storage:size:)  
7.00 M   0.9%  -                                                Packet.init(destinationConnectionID:headerLength:spin:)    
133.11 M 16.9%  -                                               specialized UniqueDeque<>.reserveCapacity(_:)   
133.11 M 16.9%  -                                                specialized RigidDeque<>.reserveCapacity(_:)   
133.11 M 16.9%  -                                                 specialized RigidDeque<>.reallocate(capacity:)    
133.11 M 16.9%  4.57 M                                             specialized _UnsafeDequeHandle<>.reallocate(capacity:)   
99.84 M  12.6%  10.00 M                                         static Deserializer<>.deserialize<>(_:claim:_:) 
50.82 M  6.4%  9.00 M                                           Frame.claim(fromStart:fromEnd:adjustSingleIPAggregate:)    
34.82 M  4.4%  34.82 M                                           0x192be0ce8 (libsystem_pthread.dylib +0x1ce9) <56F332B8-6B78-3DB7-A58C-F3F4EC03D383>  
4.00 M   0.5%  4.00 M                                            Frame.startOffset.setter  
3.00 M   0.4%  3.00 M                                            DYLD-STUB$$type metadata accessor for Logger  
23.00 M  2.9%  22.00 M                                          Frame.bytes.getter 
1.00 M   0.1%  -                                                 specialized UniqueArray<>.span.getter 
12.01 M  1.5%  -                                                static Deserializer<>.deserialize<>(_:_:)  
12.01 M  1.5%  3.01 M                                            specialized static Deserializer<>.deserialize(_:_:)   
4.00 M   0.5%  4.00 M                                           type metadata accessor for Logger

And with this change:

158.20 M 47.8%  20.74 M                                           specialized PacketParser.parseHeader(frame:dcidLength:)  
120.46 M 36.4%  149.52 k                                            PacketParser.parseShortHeader(frame:firstOctet:dcidLength:originalLength:)  
120.31 M 36.3%  50.91 M                                              specialized PacketParser.parseShortHeader(frame:firstOctet:dcidLength:originalLength:) 
44.43 M  13.4%  11.91 M                                               Frame.copyInto(inlineArray:length:)   
9.00 M   2.7%  9.00 M                                                Frame.unclaimedLength.getter  
7.00 M   2.1%  -                                                     static InlineArray<>.empty.getter 
5.00 M   1.5%  5.00 M                                                Frame.claim(fromStart:fromEnd:adjustSingleIPAggregate:)   
3.97 M   1.2%  -                                                     Packet.init(destinationConnectionID:headerLength:spin:)   
16.00 M  4.8%  -                                                   Frame.firstOctet.getter 
1.00 M   0.3%  -                                                   Frame.claim(fromStart:fromEnd:adjustSingleIPAggregate:) 

[tfpauly]

Is there an approach we can use to make the actual deserialize operation cheaper here, as opposed to avoiding using it?

[agnosticdev]

Is there an approach we can use to make the actual deserialize operation cheaper here, as opposed to avoiding using it?

I created a new type called InlineDeserializer that extracts bytes from the Frame's NetworkUniqueArray<UInt8>, this is the most performant way to extract bytes from the frame. This should get us where we want to be CPU-wise.

[agnosticdev]

Its also worth mentioning why I created a new type here (InlineDeserializer) instead of using the Deserializer type directly. The Deserializer type loads the Span<UInt8> from a frame into the Span of the Deserializer. Then when values are extracted from the Deserializer it reads them from this span. This path uses too much CPU. Extracting the bytes directly from the NetworkUniqueArray<UInt8> is by far the more performant route. After this PR I will plan to move more of our data path packet parsing to use this technique.

[tfpauly]

Interesting. Operating on a Span shouldn't be significantly more expensive than operating directly on the owned bytes. We should investigate this more.

[agnosticdev]

Interesting. Operating on a Span shouldn't be significantly more expensive than operating directly on the owned bytes. We should investigate this more.

The reason it’s expensive is due to the operations needed to create the span that the Deserializer works with a frame. Those operations are:

1. Creating a span to be inserted into the Deserializers span through the factory deserialize() method.
2. Performing operations on the Deserializers span to read values.

And when you load this span into the Deserializer for multiple frame parsing routines the CPU starts adding up.

When you use FrameDeserializer it just operates on the _bytes at a given index. So even if you did multiple reads from the frame in different places in the code the CPU impact is minimal. This should be very close to how C or Rust reads bytes from a buffer at a given index.

[tfpauly]

Accessing the span/length/etc on the frame for each field, instead of doing it once for multiple fields, seems like it would be worse for efficiency in general.

[agnosticdev]

Accessing the length is roughly the same as what hasRoom is doing in readFixedSize. Accessing the span directly on frame._bytes is cheaper than computing the RawSpan from the frame that the Deserializer uses to read values.

Just to be sure I put together a benchmark that measures parsing 10,000,000 frames with FrameDeserializer against Deserializer.deserialize.

With Deserializer.deserialize we get:

1.90 G 100.0%	-	 DeserializerBenchmark (76446)	

And with FrameDeserializer we get:

279.77 M 100.0%	-	 DeserializerBenchmark (78442)	

So that's almost 7x more CPU parsing frames with the Deserializer.deserialize approach.
Here is how I parsed the frames with FrameDeserializer:

let iterationCount = 10_000_000
var frame = Frame(copyBuffer: testBytes)
defer { frame.finalize(success: false) }
for _ in 0..<iterationCount {
    _ = try? FrameDeserializer.uint8(frame: &frame, claim: false)
    _ = try? FrameDeserializer.uint16NetworkByteOrder(frame: &frame, claim: false)
    _ = try? FrameDeserializer.uint32NetworkByteOrder(frame: &frame, claim: false)
    _ = try? FrameDeserializer.uint64NetworkByteOrder(frame: &frame, claim: false)
    _ = try? FrameDeserializer.uint8(frame: &frame, claim: false)
}

And with Deserializer.deserialize:

let iterationCount = 10_000_000
var frame = Frame(copyBuffer: testBytes)
defer { frame.finalize(success: false) }
for _ in 0..<iterationCount {
    var f1: UInt8 = 0
    var f2: UInt16 = 0
    var f3: UInt32 = 0
    var f4: UInt64 = 0
    var f5: UInt8 = 0
    _ = Deserializer.deserialize(&frame, claim: false) { read throws(DeserializationError) in
        try read.uint8(&f1)
        try read.uint16NetworkByteOrder(&f2)
        try read.uint32NetworkByteOrder(&f3)
        try read.uint64NetworkByteOrder(&f4)
        try read.uint8(&f5)
    }
}

[tfpauly]

I added a couple "always inline" marks to some of the private functions in Deserializer, and that alone changed the overall CPU time from 1.5G to 665M. So I think we can and should take the approach of optimizing the main deserializer path here. Forking to make things more specific to frame is more complex to read and is not going in the direction we want for being able to share parsing code.

[agnosticdev]

That’s great to hear but that is not enough. The main Deserializer has patterns we need to get away from if we want to compete with the performance of C or Rust. Namely, building a Span (bytes) and copying it to the Deserializer’s storage each time. This pattern cost way too much CPU when all you want to do is read a few bytes. We need to refactor that pattern in the Deserializer, and to do that I suspect we’d have to either do one of two things; one, rebuild the entire type from the ground up, or two switch to a new performant type.

[agnosticdev]

Sharing parsing code should be secondary to performance.

[tfpauly]

That is not doing either a memmove or a copy there, it is grabbing a pointer. We can look at ways to ensure the span view creation is optimized by the compiler.

[agnosticdev]

The point I am making here is that we should not even build this span in the Frame and have the Deserializer reference it. This uses too much CPU. Instead we should do the parsing directly on the Frame.

[tfpauly]

I disagree with that analysis. Let's discuss next week.

[tfpauly]

I was able to just hit 243M for this same benchmark using the span in normal deserializer, with a few other optimizations in deserializer that will apply to all existing usage.

[agnosticdev]

Lets discuss in detail next week your optimizations.

[tfpauly]

Note that this check is incorrect — it's not checking the remaining bytes after the cursor, but the whole underlying buffer size.

All of the other functions also won't work correctly in the non-claiming mode, since they have no way of tracking cursor offsets between calls.

I'm not suggesting you fix this, since this I don't think this PR has the right approach right now, but calling it out.

[agnosticdev]

Yep, this is a good call out and I would need to check the frame's unclaimed length here instead of the bytes.