Skip to content

Commit c6afe04

Browse files
Avoid CoW while processing the next state when HTTP2FrameDecoder decodes (#438)
Motivation: CoWs appear when switching over the current state of the parser and holding it while also modifying it - inside `processNExtState()`. Following `append(bytes: ByteBuffer)`'s pattern, we should use a function that sets the state to an intermediary one with no associated data, before making the transformations. Modifications: - created the `avoidParserCoW()` helper function for the throwing functions - used it in the switch cases inside `processNextState()` Result: CoW will be avoided when changing the state of the HTTP2FrameDecoder when decoding, so we won't have unnecessary heap allocations.
1 parent a0c580b commit c6afe04

File tree

1 file changed

+168
-64
lines changed

1 file changed

+168
-64
lines changed

Sources/NIOHTTP2/HTTP2FrameParser.swift

Lines changed: 168 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -781,60 +781,117 @@ struct HTTP2FrameDecoder {
781781

782782
}
783783

784+
fileprivate enum ProcessAccumulatingResult {
785+
case `continue`
786+
case needMoreData
787+
case parseFrame(header: FrameHeader, payloadBytes: ByteBuffer, paddingBytes: Int?)
788+
}
789+
784790
private mutating func processNextState() throws -> ParseResult {
785791
switch self.state {
786792
case .awaitingClientMagic(var state):
787-
guard let newState = try state.process() else {
788-
return .needMoreData
793+
return try self.avoidingParserCoW { newState in
794+
let result = Result<ParseResult, Error> {
795+
guard let processResult = try state.process() else {
796+
newState = .awaitingClientMagic(state)
797+
return .needMoreData
798+
}
799+
newState = .accumulatingFrameHeader(processResult)
800+
return .continue
801+
}
802+
switch result {
803+
case let .success(parseResult):
804+
return parseResult
805+
case let .failure(error):
806+
newState = .awaitingClientMagic(state)
807+
throw error
808+
}
789809
}
790-
791-
self.state = .accumulatingFrameHeader(newState)
792-
return .continue
793-
810+
794811
case .initialized:
795812
// no bytes, no frame
796813
return .needMoreData
797814

798815
case .accumulatingFrameHeader(var state):
799-
guard let targetState = try state.process(maxFrameSize: self.maxFrameSize, maxHeaderListSize: self.headerDecoder.maxHeaderListSize) else {
800-
return .needMoreData
816+
let maxFrameSize = self.maxFrameSize
817+
let maxHeaderListSize = self.headerDecoder.maxHeaderListSize
818+
return try self.avoidingParserCoW { newState in
819+
let result = Result<ParseResult, Error> {
820+
guard let targetState = try state.process(maxFrameSize: maxFrameSize, maxHeaderListSize: maxHeaderListSize) else {
821+
newState = .accumulatingFrameHeader(state)
822+
return .needMoreData
823+
}
824+
825+
newState = .init(targetState)
826+
return .continue
827+
}
828+
829+
switch result {
830+
case let .success(processResult):
831+
return processResult
832+
case let .failure(error):
833+
newState = .accumulatingFrameHeader(state)
834+
throw error
835+
}
801836
}
802-
803-
self.state = .init(targetState)
804-
return .continue
805-
837+
806838
case .awaitingPaddingLengthByte(var state):
807-
guard let targetState = try state.process() else {
808-
return .needMoreData
839+
return try self.avoidingParserCoW { newState in
840+
let result = Result<ParseResult, Error> {
841+
guard let targetState = try state.process() else {
842+
newState = .awaitingPaddingLengthByte(state)
843+
return .needMoreData
844+
}
845+
846+
newState = .init(targetState)
847+
return .continue
848+
}
849+
switch result {
850+
case let .success(processResult):
851+
return processResult
852+
case let .failure(error):
853+
newState = .awaitingPaddingLengthByte(state)
854+
throw error
855+
}
809856
}
810-
811-
self.state = .init(targetState)
812-
return .continue
813-
814-
857+
815858
case .accumulatingPayload(var state):
816-
guard let processResult = try state.process() else {
817-
return .needMoreData
859+
let accumulatingResult: ProcessAccumulatingResult = try self.avoidingParserCoW { newState in
860+
let result = Result<ProcessAccumulatingResult, Error> {
861+
guard let processResult = try state.process() else {
862+
newState = .accumulatingPayload(state)
863+
return .needMoreData
864+
}
865+
866+
switch processResult {
867+
case .accumulateHeaderBlockFragments(let nextState):
868+
newState = .accumulatingHeaderBlockFragments(nextState)
869+
return .continue
870+
case .parseFrame(header: let header, payloadBytes: let payloadBytes, paddingBytes: let paddingBytes, nextState: let nextState):
871+
// Save off the state first, because if the frame payload parse throws we want to be able to skip over the frame.
872+
newState = .accumulatingFrameHeader(nextState)
873+
return .parseFrame(header: header, payloadBytes: payloadBytes, paddingBytes: paddingBytes)
874+
}
875+
}
876+
877+
switch result {
878+
case let .success(processAccumulatingResult):
879+
return processAccumulatingResult
880+
case let .failure(error):
881+
newState = .accumulatingPayload(state)
882+
throw error
883+
}
818884
}
819-
820-
switch processResult {
821-
case .accumulateHeaderBlockFragments(let state):
822-
self.state = .accumulatingHeaderBlockFragments(state)
823-
return .continue
824-
825-
case .parseFrame(header: let header, payloadBytes: var payloadBytes, paddingBytes: let paddingBytes, nextState: let nextState):
826-
// Save off the state first, because if the frame payload parse throws we want to be able to skip over the frame.
827-
self.state = .accumulatingFrameHeader(nextState)
828-
885+
886+
switch accumulatingResult {
887+
case .parseFrame(header: let header, payloadBytes: var payloadBytes, paddingBytes: let paddingBytes):
829888
// an entire frame's data, including HEADERS/PUSH_PROMISE with the END_HEADERS flag set
830889
// this may legitimately return nil if we ignore the frame
831890
let result = try self.readFrame(withHeader: header, from: &payloadBytes, paddingBytes: paddingBytes)
832-
833891
if payloadBytes.readableBytes > 0 {
834892
// Enforce that we consumed all the bytes.
835893
throw InternalError.codecError(code: .frameSizeError)
836894
}
837-
838895
// if we got a frame, return it. If not that means we consumed and ignored a frame, so we
839896
// should go round again.
840897
// We cannot emit DATA frames from here, so the flow controlled length is always 0.
@@ -843,38 +900,70 @@ struct HTTP2FrameDecoder {
843900
} else {
844901
return .continue
845902
}
903+
case .continue:
904+
return .continue
905+
case .needMoreData:
906+
return .needMoreData
846907
}
847908

848909
case .simulatingDataFrames(var state):
849-
guard let processResult = try state.process() else {
850-
return .needMoreData
910+
return try self.avoidingParserCoW { newState in
911+
let result = Result<ParseResult, Error> {
912+
guard let processResult = try state.process() else {
913+
newState = .simulatingDataFrames(state)
914+
return .needMoreData
915+
}
916+
newState = .init(processResult.nextState)
917+
return .frame(processResult.frame, flowControlledLength: processResult.flowControlledLength)
918+
}
919+
switch result {
920+
case let .success(processResult):
921+
return processResult
922+
case let .failure(error):
923+
newState = .simulatingDataFrames(state)
924+
throw error
925+
}
851926
}
852927

853-
self.state = .init(processResult.nextState)
854-
return .frame(processResult.frame, flowControlledLength: processResult.flowControlledLength)
855-
856928
case .strippingTrailingPadding(var state):
857-
guard let nextState = state.process() else {
858-
return .needMoreData
929+
return self.avoidingParserCoW { newState in
930+
guard let nextState = state.process() else {
931+
newState = .strippingTrailingPadding(state)
932+
return .needMoreData
933+
}
934+
935+
newState = .init(nextState)
936+
return .continue
859937
}
860938

861-
self.state = .init(nextState)
862-
return .continue
863-
864939
case .accumulatingContinuationPayload(var state):
865-
guard let processResult = try state.process() else {
866-
return .needMoreData
940+
let accumulatingResult: ProcessAccumulatingResult = try self.avoidingParserCoW { newState in
941+
let result = Result<ProcessAccumulatingResult, Error> {
942+
guard let processResult = try state.process() else {
943+
newState = .accumulatingContinuationPayload(state)
944+
return .needMoreData
945+
}
946+
switch processResult {
947+
case .accumulateContinuationHeader(let nextState):
948+
newState = .accumulatingHeaderBlockFragments(nextState)
949+
return .continue
950+
case .parseFrame(header: let header, payloadBytes: let payloadBytes, paddingBytes: let paddingBytes, nextState: let nextState):
951+
// Save off the state first, because if the frame payload parse throws we want to be able to skip over the frame.
952+
newState = .accumulatingFrameHeader(nextState)
953+
return .parseFrame(header: header, payloadBytes: payloadBytes, paddingBytes: paddingBytes)
954+
}
955+
}
956+
switch result {
957+
case let .success(processAccumulatingResult):
958+
return processAccumulatingResult
959+
case let .failure(error):
960+
newState = .accumulatingContinuationPayload(state)
961+
throw error
962+
}
867963
}
868-
869-
switch processResult {
870-
case .accumulateContinuationHeader(let state):
871-
self.state = .accumulatingHeaderBlockFragments(state)
872-
return .continue
873-
874-
case .parseFrame(header: let header, payloadBytes: var payloadBytes, paddingBytes: let paddingBytes, nextState: let nextState):
875-
// Save off the state first, because if the frame payload parse throws we want to be able to skip over the frame.
876-
self.state = .accumulatingFrameHeader(nextState)
877-
964+
965+
switch accumulatingResult {
966+
case .parseFrame(header: let header, payloadBytes: var payloadBytes, paddingBytes: let paddingBytes):
878967
// an entire frame's data, including HEADERS/PUSH_PROMISE with the END_HEADERS flag set
879968
// this may legitimately return nil if we ignore the frame
880969
let result = try self.readFrame(withHeader: header, from: &payloadBytes, paddingBytes: paddingBytes)
@@ -883,7 +972,6 @@ struct HTTP2FrameDecoder {
883972
// Enforce that we consumed all the bytes.
884973
throw InternalError.codecError(code: .frameSizeError)
885974
}
886-
887975
// if we got a frame, return it. If not that means we consumed and ignored a frame, so we
888976
// should go round again.
889977
// We cannot emit DATA frames from here, so the flow controlled length is always 0.
@@ -892,16 +980,32 @@ struct HTTP2FrameDecoder {
892980
} else {
893981
return .continue
894982
}
983+
case .continue:
984+
return .continue
985+
case .needMoreData:
986+
return .needMoreData
895987
}
896988

897989
case .accumulatingHeaderBlockFragments(var state):
898-
guard let processResult = try state.process(maxHeaderListSize: self.headerDecoder.maxHeaderListSize) else {
899-
return .needMoreData
990+
let maxHeaderListSize = self.headerDecoder.maxHeaderListSize
991+
return try self.avoidingParserCoW { newState in
992+
let result = Result<ParseResult, Error> {
993+
guard let processResult = try state.process(maxHeaderListSize: maxHeaderListSize) else {
994+
newState = .accumulatingHeaderBlockFragments(state)
995+
return .needMoreData
996+
}
997+
newState = .accumulatingContinuationPayload(processResult)
998+
return .continue
999+
}
1000+
switch result {
1001+
case let .success(processResult):
1002+
return processResult
1003+
case let .failure(error):
1004+
newState = .accumulatingHeaderBlockFragments(state)
1005+
throw error
1006+
}
9001007
}
901-
902-
self.state = .accumulatingContinuationPayload(processResult)
903-
return .continue
904-
1008+
9051009
case .appending:
9061010
preconditionFailure("Attempting to process in appending state")
9071011
}
@@ -1346,13 +1450,13 @@ extension HTTP2FrameDecoder {
13461450
/// Sadly, because it's generic and has a closure, we need to force it to be inlined at all call sites, which is
13471451
/// not ideal.
13481452
@inline(__always)
1349-
private mutating func avoidingParserCoW<ReturnType>(_ body: (inout ParserState) -> ReturnType) -> ReturnType {
1453+
private mutating func avoidingParserCoW<ReturnType>(_ body: (inout ParserState) throws -> ReturnType) rethrows -> ReturnType {
13501454
self.state = .appending
13511455
defer {
13521456
assert(!self.isAppending)
13531457
}
13541458

1355-
return body(&self.state)
1459+
return try body(&self.state)
13561460
}
13571461

13581462
private var isAppending: Bool {

0 commit comments

Comments
 (0)