search

apple / swift-nio-http2

HTTP/2 support for SwiftNIO
https://swiftpackageindex.com/apple/swift-nio-http2/main/documentation/niohttp2
Apache License 2.0
465 stars 82 forks source link

Reduce allocations on `InlineStreamMultiplexer/createStreamChannel` #450

Closed gjcairo closed 3 months ago

gjcairo commented 4 months ago

Motivation

When creating stream channels on the InlineStreamMultiplexer, we are currently performing a flatSubmit on two different codepaths. This allocates two ELFs: one on submit, and one on flatMap. This is unnecessary, as we could do with just one allocation.

Modifications

Create a single ELP and call execute on the EL instead of using flatSubmit.

Result

Fewer allocations

glbrntt commented 4 months ago

@swift-server-bot add to allowlist

gjcairo commented 3 months ago

@swift-server-bot test this please

gjcairo commented 3 months ago

I updated the allocations for nightly because they had been outdated for a while.

However this change doesn't affect any allocation tests: this is because allocation tests that call createStreamChannel, call the createStreamChannel(promise:_:) version (instead of createStreamChannel(_:)), which is unaffected by this change. We could duplicate the allocation tests to use createStreamChannel(_:), or modify them to stop using the version with the promise, but I don't know if that's worth it/desired?

glbrntt commented 3 months ago

However this change doesn't affect any allocation tests: this is because allocation tests that call createStreamChannel, call the createStreamChannel(promise:_:) version (instead of createStreamChannel(_:)), which is unaffected by this change. We could duplicate the allocation tests to use createStreamChannel(_:), or modify them to stop using the version with the promise, but I don't know if that's worth it/desired?

I think it's probably worth adding a version of the test which doesn't pass in the promise in a separate PR and verifying that we do save allocations here.