Skip to content

More cleanup in client_channel code.#12297

Merged
markdroth merged 1 commit intogrpc:masterfrom
markdroth:moar_client_channel_cleanup
Sep 11, 2017
Merged

More cleanup in client_channel code.#12297
markdroth merged 1 commit intogrpc:masterfrom
markdroth:moar_client_channel_cleanup

Conversation

@markdroth
Copy link
Copy Markdown
Member

@markdroth markdroth commented Aug 25, 2017

This slightly simplifies the pick logic.

@markdroth markdroth requested a review from dgquintas August 25, 2017 20:05
@markdroth markdroth requested a review from ctiller as a code owner August 25, 2017 20:05
@grpc-kokoro
Copy link
Copy Markdown
Collaborator

grpc-kokoro commented Aug 25, 2017 

[trickle] No significant performance differences

@grpc-kokoro
Copy link
Copy Markdown
Collaborator

grpc-kokoro commented Aug 25, 2017 

[microbenchmarks] No significant performance differences

@markdroth markdroth mentioned this pull request Aug 25, 2017
Merged
@markdroth markdroth mentioned this pull request Aug 29, 2017
Closed
@markdroth markdroth force-pushed the moar_client_channel_cleanup branch from 0facb08 to 4ac9fe9 Compare September 8, 2017 14:51
@grpc-kokoro
Copy link
Copy Markdown
Collaborator

grpc-kokoro commented Sep 8, 2017 

[microbenchmarks] No significant performance differences

@grpc-kokoro
Copy link
Copy Markdown
Collaborator

grpc-kokoro commented Sep 8, 2017 

Corrupt JSON data (indicates timeout or crash): 
    bm_fullstack_trickle.BM_PumpStreamServerToClient_Trickle_2M_32k.opt.new: 1


[trickle] No significant performance differences

@markdroth markdroth force-pushed the moar_client_channel_cleanup branch from 4ac9fe9 to b292960 Compare September 11, 2017 16:31
@grpc-kokoro
Copy link
Copy Markdown
Collaborator

grpc-kokoro commented Sep 11, 2017 

[trickle] No significant performance differences

@grpc-kokoro
Copy link
Copy Markdown
Collaborator

grpc-kokoro commented Sep 11, 2017 

[microbenchmarks] Performance differences noted:
Benchmark                                                              cpu_time    real_time
---------------------------------------------------------------------  ----------  -----------
BM_PumpStreamClientToServer<InProcess>/256k                            +23%        +22%
BM_PumpStreamClientToServer<InProcess>/32k                             +9%         +9%
BM_PumpStreamClientToServer<InProcess>/4k                              +4%         +4%
BM_PumpStreamClientToServer<InProcessCHTTP2>/32k                                   +4%
BM_PumpStreamServerToClient<InProcess>/256k                            +6%         +7%
BM_PumpStreamServerToClient<InProcessCHTTP2>/32k                                   +4%
BM_UnaryPingPong<InProcess, NoOpMutator, NoOpMutator>/0/256k           +8%         +8%
BM_UnaryPingPong<InProcess, NoOpMutator, NoOpMutator>/256k/0           +13%        +14%
BM_UnaryPingPong<InProcess, NoOpMutator, NoOpMutator>/256k/256k        +6%         +6%
BM_UnaryPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/256k     +9%         +10%
BM_UnaryPingPong<MinInProcess, NoOpMutator, NoOpMutator>/0/256k        +11%        +11%
BM_UnaryPingPong<MinInProcess, NoOpMutator, NoOpMutator>/256k/0        +8%         +7%
BM_UnaryPingPong<MinInProcess, NoOpMutator, NoOpMutator>/32k/32k       +6%         +6%
BM_UnaryPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/256k/0  +4%         +4%
BM_UnaryPingPong<MinTCP, NoOpMutator, NoOpMutator>/256k/256k           +4%         +5%

@markdroth
Copy link
Copy Markdown
Member Author

markdroth commented Sep 11, 2017

I suspect those bm_diff numbers are spurious. I'm going to trigger a re-run to see if they recur.

@grpc-kokoro
Copy link
Copy Markdown
Collaborator

grpc-kokoro commented Sep 11, 2017 

[microbenchmarks] No significant performance differences

@markdroth markdroth mentioned this pull request Sep 11, 2017
Closed
@markdroth
Copy link
Copy Markdown
Member Author

markdroth commented Sep 11, 2017

Known issues: #11737 #11676 #11745 #12490

@markdroth markdroth merged commit 1b96418 into grpc:master Sep 11, 2017
@markdroth markdroth deleted the moar_client_channel_cleanup branch September 11, 2017 22:25
markdroth added a commit to markdroth/grpc that referenced this pull request Oct 23, 2017
@markdroth
Restore logic from before grpc#12297.
48be9de
markdroth added a commit that referenced this pull request Oct 24, 2017
@markdroth
Merge pull request #13070 from markdroth/client_channel_lb_policy_ref…
c263204 
…_fix

Restore pick-after-resolver-result logic from before #12297.
@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 2019
@lock lock bot unassigned dgquintas Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ctiller ctiller Awaiting requested review from ctiller

1 more reviewer

@dgquintas dgquintas dgquintas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@markdroth @grpc-kokoro @dgquintas @googlebot