-
Notifications
You must be signed in to change notification settings - Fork 420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an Async Ping Pong streaming benchmark #1004
Conversation
Motivation: Unary benchmark already exists - adding a streaming benchmark completes very high level coverage. Modifications: Refactor Unary benchmaark to break out what's specific to that and add a second implementation of that broken out class which runs a streaming test. Result: Both Unary and Streaming benchmarks exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good: we can avoid allocating promises in a couple of places so we should do that.
self.messagesPerStream == 0 || messagesSent < self.messagesPerStream { | ||
messagesSent += 1 | ||
startTime = endTime // Use end of previous request as the start of the next. | ||
_ = streamingCall!.sendMessage(self.requestMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can avoid allocating the promise here: sendMessage(self.requestMessage, promise: nil)
Performance/QPSBenchmark/Sources/QPSBenchmark/Runtime/AsyncPingPongRequestMaker.swift
Show resolved
Hide resolved
// Setup the call. | ||
streamingCall = self.client.streamingCall(handler: handleResponse) | ||
// Kick start with initial request | ||
_ = streamingCall!.sendMessage(self.requestMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid the promise: promise: nil
Co-authored-by: George Barnett <[email protected]>
Motivation:
Unary benchmark already exists - adding a streaming benchmark
completes very high level coverage.
Modifications:
Refactor Unary benchmaark to break out what's specific to
that and add a second implementation of that broken out
class which runs a streaming test.
Result:
Both Unary and Streaming benchmarks exist.