From a0224f3d20438635dd59c9fcc593520d80d131d0 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Sat, 29 Jun 2024 13:20:48 +0100 Subject: [PATCH] Discard read bytes when accumulating continuation frames (#444) Motivation: When accumulating sequences of CONTINUATION frames, each frame is parsed from a buffer. These bytes are read when the CONTINUATION frame is parsed, but if more CONTINUATION frames follow then the buffer isn't reset. This means that long sequences of CONTINUATION frames can result in a larger than necessary buffer where most of the contents have already been read. Modifications: - Discard the bytes of the accumulation buffer when transitioning back to AccumulatingHeaderBlockFragmentsParserState if more than half of the buffer has been read. Result: Lower memory footprint when parsing sequences of CONTINUATION frames. --- Sources/NIOHTTP2/HTTP2FrameParser.swift | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Sources/NIOHTTP2/HTTP2FrameParser.swift b/Sources/NIOHTTP2/HTTP2FrameParser.swift index 84f01202..3ba6066d 100644 --- a/Sources/NIOHTTP2/HTTP2FrameParser.swift +++ b/Sources/NIOHTTP2/HTTP2FrameParser.swift @@ -537,6 +537,19 @@ struct HTTP2FrameDecoder { // we have collected enough bytes: is this the last CONTINUATION frame? guard self.continuationHeader.flags.contains(.endHeaders) else { + // When accumulating a sequence of CONTINUATION frames the buffer will continue to + // grow as bytes are appended to it and the reader index will continue to advance + // as frames are read. However this allows for an ever increasing buffer where only + // the unread bytes are useful. The read bytes should be periodically discarded. + // + // As a basic heuristic is to discard the read bytes when more than 1KB has been + // read, and that 1KB accounts for more than half of the capacity. + let readerIndex = self.continuationPayload.readerIndex + let capacity = self.continuationPayload.capacity + if readerIndex > 1024 && readerIndex > (capacity / 2) { + self.continuationPayload.discardReadBytes() + } + // nope, switch back to accumulating fragments return .accumulateContinuationHeader( AccumulatingHeaderBlockFragmentsParserState(