Skip to content

stats: Re-use objects while calling multiple Handlers#8639

Merged
arjan-bal merged 5 commits intogrpc:masterfrom
arjan-bal:stats-handlers-reuse-event-objects
Oct 17, 2025
Merged

stats: Re-use objects while calling multiple Handlers#8639
arjan-bal merged 5 commits intogrpc:masterfrom
arjan-bal:stats-handlers-reuse-event-objects

Conversation

@arjan-bal
Copy link
Copy Markdown
Contributor

@arjan-bal arjan-bal commented Oct 9, 2025 •
edited
Loading

This PR improves performance by eliminating heap allocations when multiple stats handlers are configured.

Previously, iterating through a list of handlers caused one heap allocation per handler for each RPC. This change introduces a Handler that combines multiple Handlers and implements the Handler interface. The combined handler delegates calls to the handlers it contains.

This approach allows gRPC clients and servers to operate as if there were only a single Handler registered, simplifying the internal logic and removing the per-RPC allocation overhead. To avoid any performance impact when stats are disabled, the combined Handler is only created when at least one handler is registered.

Tested

Since existing benchmarks don't register stats handler, I modified the benchmark to add 2 stats handlers each on the server and client (36ba616).

# test command
go run benchmark/benchmain/main.go -benchtime=60s -workloads=unary \
   -compression=off -maxConcurrentCalls=200 -trace=off \
   -reqSizeBytes=100 -respSizeBytes=100 -networkMode=Local -resultFile="${RUN_NAME}"

# results
go run benchmark/benchresult/main.go unary-before unary-after
               Title       Before        After Percentage
            TotalOps      7336128      7638892     4.13%
             SendOps            0            0      NaN%
             RecvOps            0            0      NaN%
            Bytes/op     12382.19     11467.03    -7.39%
           Allocs/op       173.93       165.91    -4.60%
             ReqT/op  97815040.00 101851893.33     4.13%
            RespT/op  97815040.00 101851893.33     4.13%
            50th-Lat   1.463345ms   1.403011ms    -4.12%
            90th-Lat   2.557136ms    2.46828ms    -3.47%
            99th-Lat   3.073264ms   3.080081ms     0.22%
             Avg-Lat   1.634153ms   1.569391ms    -3.96%
           GoVersion     go1.24.7     go1.24.7
         GrpcVersion   1.77.0-dev   1.77.0-dev

RELEASE NOTES:

  • stats: Reduce heap allocations when multiple stats Handlers are registered.

@arjan-bal arjan-bal changed the title stasts: Re-use objects while calling multiple Handlers stats: Re-use objects while calling multiple Handlers Oct 9, 2025
@arjan-bal arjan-bal added Type: Performance Performance improvements (CPU, network, memory, etc) Area: Observability Includes Stats, Tracing, Channelz, Healthz, Binlog, Reflection, Admin, GCP Observability labels Oct 9, 2025
@arjan-bal arjan-bal modified the milestone: 1.77 Release Oct 9, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 9, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 98.55072% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.00%. Comparing base (0e9ac5d) to head (197815c).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
internal/transport/http2_client.go 90.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8639      +/-   ##
==========================================
+ Coverage   81.28%   82.00%   +0.72%     
==========================================
  Files         415      417       +2     
  Lines       40888    40793      -95     
==========================================
+ Hits        33234    33452     +218     
+ Misses       6162     5981     -181     
+ Partials     1492     1360     -132
Files with missing lines Coverage Δ
clientconn.go 90.08% <100.00%> (+0.27%) ⬆️
internal/stats/stats.go 100.00% <100.00%> (ø)
internal/transport/handler_server.go 90.85% <100.00%> (+<0.01%) ⬆️
internal/transport/http2_server.go 90.57% <100.00%> (-0.57%) ⬇️
internal/transport/transport.go 90.72% <ø> (+6.85%) ⬆️
server.go 81.98% <100.00%> (-0.10%) ⬇️
stream.go 81.60% <100.00%> (+0.03%) ⬆️
internal/transport/http2_client.go 92.36% <90.47%> (+8.22%) ⬆️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arjan-bal arjan-bal force-pushed the stats-handlers-reuse-event-objects branch 2 times, most recently from a36c3d1 to 88fb8c2 Compare October 9, 2025 16:37
@arjan-bal arjan-bal force-pushed the stats-handlers-reuse-event-objects branch from 88fb8c2 to 38cc04a Compare October 9, 2025 16:39
@arjan-bal arjan-bal requested a review from easwars October 9, 2025 16:49
@arjan-bal arjan-bal requested a review from dfawley October 9, 2025 18:21
@easwars
Copy link
Copy Markdown
Contributor

easwars commented Oct 10, 2025

Do you have any benchmark numbers for this change?

easwars
easwars reviewed Oct 10, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@easwars easwars left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Also, I was wondering what if you exported the combinedHandler and used it as a concrete type instead of as stats.Handler? The grpc server and the channel could use this concrete type, and any other internal type could as well. For external facing APIs that are expecting a stats.Handler, it should still continue to work if the caller has the concrete type. But again, I don't know if this will have any bearing on the performance. So, if this is going to increase the scope of the PR, please push back. Thanks.

Comment thread internal/transport/http2_client.go
Comment thread internal/stats/stats.go
Comment thread internal/transport/http2_client.go
Comment thread clientconn.go Outdated
@easwars easwars assigned arjan-bal and unassigned easwars and dfawley Oct 10, 2025
@arjan-bal
Copy link
Copy Markdown
Contributor Author

arjan-bal commented Oct 16, 2025

Do you have any benchmark numbers for this change?

I had to temporarily modify the benchmarks to register stats handlers on the client and server. I've added the results in the PR description.

@arjan-bal
Copy link
Copy Markdown
Contributor Author

arjan-bal commented Oct 16, 2025

Also, I was wondering what if you exported the combinedHandler and used it as a concrete type instead of as stats.Handler? The grpc server and the channel could use this concrete type, and any other internal type could as well. For external facing APIs that are expecting a stats.Handler, it should still continue to work if the caller has the concrete type. But again, I don't know if this will have any bearing on the performance. So, if this is going to increase the scope of the PR, please push back. Thanks.

I considered exporting combinedHandler and returning it as a concrete type but ultimately felt that returning the stats.Handler interface was a cleaner design. My main reasons are:

  1. Reduced Coupling: It prevents the gRPC server and channel from needing to know about the internal combinedHandler type, which feels like a more robust, long-term design.
  2. Encapsulation: It avoids exposing the implementation details of how multiple handlers are managed.
  3. Optimization: By returning the interface, we can add a small optimization: if only one stats handler is registered, we can return it directly without the overhead of wrapping it in a combinedHandler at all.

@arjan-bal arjan-bal assigned easwars and unassigned arjan-bal Oct 16, 2025
@easwars easwars assigned arjan-bal and unassigned easwars Oct 16, 2025
@arjan-bal arjan-bal merged commit ab1ca08 into grpc:master Oct 17, 2025
15 checks passed
@arjan-bal arjan-bal deleted the stats-handlers-reuse-event-objects branch October 17, 2025 06:49
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@easwars easwars easwars approved these changes

@dfawley dfawley Awaiting requested review from dfawley

Assignees

@arjan-bal arjan-bal

Labels

Area: Observability Includes Stats, Tracing, Channelz, Healthz, Binlog, Reflection, Admin, GCP Observability Type: Performance Performance improvements (CPU, network, memory, etc)

Projects

None yet

Milestone

1.77 Release

Development

Successfully merging this pull request may close these issues.

3 participants

@arjan-bal @easwars @dfawley