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.

Sign up for GitHub

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

Jump to bottom

Change metadata names used by the instrumentation. #10405

Merged
merged 1 commit into from
Apr 14, 2017
Merged

Change metadata names used by the instrumentation. #10405

merged 1 commit into from
Apr 14, 2017

Conversation

bogdandrutu
Copy link

No description provided.

a-veitch
a-veitch approved these changes Mar 31, 2017
View reviewed changes
Copy link
Contributor

@a-veitch a-veitch left a comment

Choose a reason for hiding this comment

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

LGTM

@dgquintas dgquintas self-assigned this Mar 31, 2017
@ctiller
Copy link
Member

ctiller commented Mar 31, 2017

You'll need to run tools/distrib/clang-format-code.sh

@grpc-kokoro
Copy link 

Performance differences noted:
Benchmark                                                                                            allocs_per_iteration    atm_add_per_iteration    atm_cas_per_iteration    cpu_time     locks_per_iteration    real_time    writes_per_iteration
---------------------------------------------------------------------------------------------------  ----------------------  -----------------------  -----------------------  -----------  ---------------------  -----------  ----------------------
BM_PumpStreamClientToServer<SockPair>/128M                                                                                                                                     -19056241.5                         -19054413.0
BM_PumpStreamClientToServer<TCP>/16M                                                                                                                                           -1752388.5                          -1751974.0
BM_PumpStreamServerToClient<InProcessCHTTP2>/128M                                                                                                                              24370379.0                          24367451.0
BM_PumpStreamServerToClient<TCP>/0                                                                                           -3.0224                  -0.9096                               -0.525505
BM_PumpStreamServerToClient<TCP>/128M                                                                                                                                          -22225988.5                         -22226492.5
BM_StreamingPingPong<TCP, NoOpMutator, NoOpMutator>/512/1                                                                                                                      -9674.0                             -9674.0
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/16M/1/1                                                                                       -3255964.0                          -3278092.5
BM_UnaryPingPong<InProcessCHTTP2, Client_AddMetadata<RandomAsciiMetadata<100>, 1>, NoOpMutator>/0/0                                                                            1018.0                              1018.0
BM_UnaryPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/4k                                                                                                               1008.0                              1008.0
BM_UnaryPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/128M/128M                                                                                                          -39586977.5                         -39591312.5
BM_UnaryPingPong<TCP, NoOpMutator, NoOpMutator>/0/16M                                                                                                                          -3232318.5                          -3233404.0
BM_UnaryPingPong<TCP, NoOpMutator, NoOpMutator>/16M/0                                                                                                 858.075
BM_UnaryPingPong<TCP, NoOpMutator, NoOpMutator>/16M/16M                                                                                               1264.1
BM_UnaryPingPong<TCP, NoOpMutator, NoOpMutator>/2M/2M                                                                                                                          937347.5                            938465.5

@bogdandrutu
Copy link
Author

Done. PTAL.

ctiller
ctiller reviewed Mar 31, 2017
View reviewed changes
Copy link
Member

@ctiller ctiller left a comment

Choose a reason for hiding this comment

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

We'll be able to write faster code if these strings also go into the METADATA_BATCH_CALLOUTS in gen_static_metadata.py

@grpc-kokoro
Copy link 

No significant performance differences

@bogdandrutu
Copy link
Author

Done.

@grpc-kokoro
Copy link 

No significant performance differences

@bogdandrutu
Copy link
Author

Any chance we can get this merged?

@grpc-kokoro
Copy link 

Performance differences noted:
Benchmark                     cpu_time    real_time
--------------------------  ----------  -----------
BM_Pass1Cpp                     +13.00       +13.00
BM_TransportStreamSend/16M   +12205.50    +12205.00
BM_TransportStreamSend/32k      +68.00       +68.00
BM_TransportStreamSend/512      +44.50       +44.50

@dgquintas
Copy link
Contributor

I'm now keeping an eye on this and will merge ASAP.

@grpc-kokoro
Copy link 

No significant performance differences

@dgquintas
Copy link
Contributor

Merging on mobile. Will comment on failures later

@dgquintas dgquintas merged commit 3bcf8d7 into grpc:master Apr 14, 2017
@dgquintas
Copy link
Contributor

Issues: #9542 #10640 #10576

@lock lock bot locked as resolved and limited conversation to collaborators Jan 23, 2019
@lock lock bot unassigned dgquintas Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@a-veitch a-veitch a-veitch approved these changes

@ctiller ctiller ctiller approved these changes

Assignees
No one assigned
Labels
area/Census cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bogdandrutu @ctiller @grpc-kokoro @dgquintas @a-veitch @googlebot