Skip to content
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

Merged
merged 6 commits into from
Oct 22, 2020

Conversation

PeterAdams-A
Copy link
Contributor

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.

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.
Copy link
Collaborator

@glbrntt glbrntt left a 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.

Performance/QPSBenchmark/README.md Outdated Show resolved Hide resolved
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)
Copy link
Collaborator

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)

// Setup the call.
streamingCall = self.client.streamingCall(handler: handleResponse)
// Kick start with initial request
_ = streamingCall!.sendMessage(self.requestMessage)
Copy link
Collaborator

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

@PeterAdams-A PeterAdams-A requested a review from glbrntt October 22, 2020 12:11
@PeterAdams-A PeterAdams-A requested a review from glbrntt October 22, 2020 13:22
@glbrntt glbrntt added the semver/none No version bump required. label Oct 22, 2020
@glbrntt glbrntt merged commit 992cf22 into grpc:main Oct 22, 2020
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