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

proposal: runtime/metrics: add mutex slow path counters #54236

Closed
bradfitz opened this issue Aug 3, 2022 · 6 comments
Closed

proposal: runtime/metrics: add mutex slow path counters #54236

bradfitz opened this issue Aug 3, 2022 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge Proposal
Milestone

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Aug 3, 2022

We modified the runtime & sync package a bit to add counters when a sync.Mutex.Lock and sync.RWMutex.RLock call go into their slow paths:

tailscale@effe2d1 (corp CLA on file; same license as Go)

That's a bit of hack (adding some global variables to the sync package) but it gets us information easily.

Would the runtime team be interested in adding this sort of information more officially via the runtime/metrics package?

We know about the "block" and "mutex" pprof types, but they're a bit more tedious to use to just get a counter.

Our use case is: we have a server that with some very hot mutexes and we want to high level metric showing how happy it is that'll become unhappy looking if people add too much code in the critical section.

@gopherbot gopherbot added this to the Proposal milestone Aug 3, 2022
@prattmic
Copy link
Member

prattmic commented Aug 3, 2022

cc @golang/runtime @mknyszek

@prattmic prattmic added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 3, 2022
@mknyszek
Copy link
Contributor

mknyszek commented Aug 3, 2022

I'm certainly not opposed!

If you have a concrete implementation in mind and want to send a patch, I'm happy to review the changes. Otherwise I'll try to get around to it this cycle, but no promises. I was planning on adding some GC CPU counters this dev cycle, along with maybe a few more scheduling metrics.

@mknyszek
Copy link
Contributor

mknyszek commented Aug 3, 2022

Also, I think there might actually be another issue about this? #49881 I also believe there was an internal feature request for this, too.

Seems like a good idea. :)

@prattmic
Copy link
Member

prattmic commented Aug 3, 2022

Just to be concrete, if I understand correctly, a mutex profile would work here, but you are looking for a general "contention level/rate" counter that can be easily monitored via a monitoring system, which could alert you that you may want to look at an actual profile.

I also think that sounds reasonable and useful.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Aug 3, 2022
@rsc
Copy link
Contributor

rsc commented Aug 3, 2022

Closing as duplicate of #49881, which seems to be slated for Go 1.20.

@seankhliao
Copy link
Member

Duplicate of #49881

@seankhliao seankhliao marked this as a duplicate of #49881 Aug 3, 2022
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Aug 3, 2022
@golang golang locked and limited conversation to collaborators Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge Proposal
Projects
No open projects
Development

No branches or pull requests

6 participants