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

Do not flush NewStream header on client side for unary RPCs and streaming RPCs with requests. #1343

Merged
merged 2 commits into from
Jul 5, 2017

Conversation

menghanl
Copy link
Contributor

@menghanl menghanl commented Jun 29, 2017

If it's not client streaming, we should already have the request to be sent, so we don't flush the header.
If it is client streaming, the user may never send a request or send it any time soon, so we ask the transport to flush the header.

This change reduced one system call for flushing the headers.

Without the change:

Unary-Tracing-c_1-req_1B-resp_1B_noMD-12         	   10000	    103777 ns/op
Unary-Tracing-c_1-req_1B-resp_1B_withMD-12       	   10000	    115799 ns/op
Unary-Tracing-c_1-req_1B-resp_1B_withBinMD-12    	   10000	    117684 ns/op

With the change:

Unary-Tracing-c_1-req_1B-resp_1B_noMD-12         	   10000	    103641 ns/op
Unary-Tracing-c_1-req_1B-resp_1B_withMD-12       	   10000	    105663 ns/op
Unary-Tracing-c_1-req_1B-resp_1B_withBinMD-12    	   10000	    109140 ns/op

For unary RPCs with metadata, latency is reduced about 10%.

@menghanl menghanl force-pushed the not_flush_header branch 2 times, most recently from 802da4f to 237931f Compare June 29, 2017 23:38
…ming RPCs with at least one request.

If it's not client streaming, we should already have the request to be sent,
so we don't flush the header.
If it's client streaming, the user may never send a request or send it any
time soon, so we ask the transport to flush the header.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@dfawley dfawley self-assigned this Jun 30, 2017
@menghanl menghanl merged commit 41d9b6e into grpc:master Jul 5, 2017
@menghanl menghanl deleted the not_flush_header branch July 5, 2017 23:51
@menghanl menghanl added 1.5 Type: Performance Performance improvements (CPU, network, memory, etc) labels Jul 10, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Performance Performance improvements (CPU, network, memory, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants