SwiftQUIC: PERF: Avoid temporary frame array in the send path

[agnosticdev]

This one is a double performance win, we save around 150 megacycles and we avoid a temporary allocation for FrameArray each time SwiftQUIC sends.
Today, each time SwiftQUIC sends frames it builds a temporary outboundFrameArray to add all of these frames in and then adds these frames to the lowerSendQueue.
We can just add these frames directly to the lowerSendQueue and take a CPU win too as long as we keep the capacity of the lowerSendQueue.

Top of tree:

29.97 G  88.8%	4.50 M  	  QUICConnection.buildSinglePacketForKeyState()	
29.87 G  88.5%	240.58 M	  specialized QUICConnection.buildSinglePacketForKeyState()	
220.14 M 0.7%	35.41 M 	  FrameArray.add(frame:)	


33.03 G 42.6%	8.00 M 	      QUICConnection.sendApplicationFrames()	
31.13 G 40.1%	44.91 M	       QUICConnection.runApplicationBurstLoop()	
1.55 G  2.0%	1.00 M 	       QUICConnection.sendOutboundFrames(_:on:)	

With this change:

29.49 G  88.5%	4.85 M  	   QUICConnection.buildSinglePacketForKeyState()	
29.44 G  88.4%	213.60 M	   specialized QUICConnection.buildSinglePacketForKeyState()	
172.28 M 0.5%	6.00 M  	   specialized ManyToManyOutboundDatagramProtocol<>.enqueueOutboundFrame(frame:path:)	

33.68 G 43.9%	1.00 M 	       QUICConnection.sendFrames()	
1.44 G  1.9%	-      	        QUICConnection.sendOutboundFrames(on:)	

[rnro]

This looks like a great perf change, I think I agree with all of Tommy's comments 😄. The methods on an object which never access self. is a giveaway for these.

[glbrntt]

count is the number elements, not the underlying capacity, is using it for the capacity intentional? Should it be self.frames.capacity instead?

[agnosticdev]

It is intentionally using count, yes. Check out the creation of lowerSendQueue, it now has a starting capacity on it too. The intention here is that these FrameArray's should maintain a capacity based on how many frames were previously in lowerSendQueue. That way the collection does not have to spend so many cycles reallocating to fit newly added frames.

[glbrntt]

So is the assumption that the lowerSendQueue sends approximately the same number of items each time?

This doesn't feel like it would be representative of real traffic patterns. and I'd be worried about one large or small batch negatively impacting perf because the next FrameArray is then badly sized. I wonder if even a simple heuristic like taking the median of the last three FrameArray sizes would be more effective?

[glbrntt]

Also: I think the comment on the function is misleading because the original capacity isn't kept. Some capacity is kept but it might not be all of it.

[agnosticdev]

So is the assumption that the lowerSendQueue sends approximately the same number of items each time?

This doesn't feel like it would be representative of real traffic patterns. and I'd be worried about one large or small batch negatively impacting perf because the next FrameArray is then badly sized. I wonder if even a simple heuristic like taking the median of the last three FrameArray sizes would be more effective?

For larger transfers, yes, the send queue should be capped at 40 packets and if the congestion window is opened wide enough then we will fill this each time we send. There can be error here, yes, but for larger transfers this methodology is effective, as seen in the data above.

Just to make sure I am not regressing performance for highly concurrent, multi-stream, traffic patterns I ran our QUICStreamLoad benchmark with this change and with top of tree, and the results are almost a wash. So for these traffic patterns we would not be regressing performance.
Top of tree:

1.25 G 49.2%	14.00 M	   specialized QUICConnection.buildSinglePacketForKeyState()	
1.72 G 16.4%	1.01 M	   specialized QUICConnection.sendApplicationFrames()	
4.00 M 100.0%	-	       drainArray

With this change

1.22 G 49.0%	14.00 M	   specialized QUICConnection.buildSinglePacketForKeyState()	
1.74 G 16.6%	-	       specialized QUICConnection.sendApplicationFrames()	
7.00 M 100.0%	-	       drainArrayKeepingCapacity

[agnosticdev]

Also: I think the comment on the function is misleading because the original capacity isn't kept. Some capacity is kept but it might not be all of it.

Removed comment in e2963a9