Skip to content

Conversation

FranzBusch
Copy link
Member

Motivation

We want to benchmark our NIOAsyncChannel to see how it compares to the synchronous implementation with ChannelHandlers

Modification

This PR adds a new TCPEchoAsyncChannel benchmark that mimics the TCPEcho benchmark but uses our new async bridges. Since Swift Concurrency, is normally using a global executor this benchmark would have quite high variation. To reduce this variant I introduced code to hook the global executor and set an EventLoop as the executor. In the future, if we get task executors we can change the code to us them instead.

Result

New baseline benchmarks for the NIOAsyncChannel.

@@ -0,0 +1,3 @@
{
"mallocCountTotal" : 5636901
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty bad since we are allocating for every single enqueue now. I will raise one or more follow up PRs to reduce this significantly. I was able to remove all allocations except one in the runtime.

# Motivation
We want to benchmark our `NIOAsyncChannel` to see how it compares to the synchronous implementation with `ChannelHandler`s

# Modification
This PR adds a new `TCPEchoAsyncChannel` benchmark that mimics the `TCPEcho` benchmark but uses our new async bridges.
Since Swift Concurrency, is normally using a global executor this benchmark would have quite high variation. To reduce this variant I introduced code to hook the global executor and set an `EventLoop` as the executor. In the future, if we get task executors we can change the code to us them instead.

# Result
New baseline benchmarks for the `NIOAsyncChannel`.
@FranzBusch FranzBusch force-pushed the fb-async-channel-benchmark branch from 8edf20d to 0cb17ae Compare October 9, 2023 07:37
import Benchmark
import NIOPosix

private let eventLoop = MultiThreadedEventLoopGroup(numberOfThreads: 1).next()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just grab the next() from the shared singleton?

}
}

let bufferSize = 10000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: naming threw me for a sec as I expected a buffer, not a ByteBuffer. something like messageSize is a more obvious

@FranzBusch
Copy link
Member Author

Thanks for the review. Going to address them in my next PR that I am going to open in a few minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants