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

reduce number of scope activations in gRPC client instrumentation #5470

Merged

Conversation

richardstartin
Copy link
Member

@richardstartin richardstartin commented Jun 26, 2023

What Does This Do

#4646 and #4680 changed the way context propagation worked in the gRPC instrumentation: rather than context being captured by SerializingExecutor.schedule (which simply executes the SerializingExecutor itself as a Runnable)
and activated by SerializingExecutor.run (which drains a queue of Runnable tasks and runs each one) the context was now captured by each task submitted to SerializingExecutor.execute and activated when each task would run. This prevents the context active when SerializingExecutor.schedule was called being active when each task runs. However, a single SerializingExecutor only ever executes tasks for the same request, so allowing SerializingExecutor.run to activate the scope once drastically reduces the number of scope activations (they are all the same scopes), thus reducing tracing overhead. This change essentially reverts #4646 and #4680 and lets SerializingExecutor.run capture and activate the same scope for all the tasks it executes.

Motivation

Reduce overhead

Additional Notes

@richardstartin richardstartin added tag: performance Performance related changes inst: grpc gRPC instrumentation labels Jun 26, 2023
@richardstartin richardstartin requested a review from a team as a code owner June 26, 2023 23:26
@richardstartin richardstartin force-pushed the rgs/gRPC-serializingexecutor-scope-activations branch from 0fbf440 to 4af53e6 Compare June 26, 2023 23:33
@pr-commenter
Copy link

pr-commenter bot commented Jun 26, 2023

Benchmarks

Parameters

Baseline Candidate
commit 1.17.0-SNAPSHOT~103ef9223f 1.17.0-SNAPSHOT~c91cb61f84
config baseline candidate
See matching parameters
Baseline Candidate
module Agent Agent
parent None None

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 22 cases.

@richardstartin richardstartin force-pushed the rgs/gRPC-serializingexecutor-scope-activations branch from 4af53e6 to c91cb61 Compare June 27, 2023 00:26
Copy link
Contributor

@bantonsson bantonsson left a comment

Choose a reason for hiding this comment

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

I assume that you're referring to #4646 and not #4676

@richardstartin richardstartin merged commit d133260 into master Jun 27, 2023
@richardstartin richardstartin deleted the rgs/gRPC-serializingexecutor-scope-activations branch June 27, 2023 07:54
@github-actions github-actions bot added this to the 1.17.0 milestone Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: grpc gRPC instrumentation tag: performance Performance related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants