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

Jump to bottom

runtime: make all waitReasons distinct? #24362

Closed
josharian opened this issue Mar 12, 2018 · 6 comments
Closed

runtime: make all waitReasons distinct? #24362

josharian opened this issue Mar 12, 2018 · 6 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.11

Comments

@josharian
Copy link
Contributor

Migrated from the discussion in CL 99078:

There are a couple of places where some reasons are already re-used. Is it worth making them distinct (in a separate CL)? Another option for making them distinct, now that we have a waitReason uint8, is giving them different numerical values but stringify them the same.

Should we do this? If so, distinct ints only or also distinct strings?

The fix is pretty easy regardless. cc @aclements for opinions.
@josharian josharian added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 12, 2018
@josharian josharian added this to the Go1.11 milestone Mar 12, 2018
@aclements
Copy link
Member

I don't think distinct ints are useful. That's a perfectly sensible encoding, but I don't ever want to show the ints to a user.

Do you have the re-used reasons handy?

@josharian
Copy link
Contributor Author 

$ grep -R -I "gopark" . | awk -F '"' '{print $2}' | grep -v "^$" | sort | uniq -c | sort -n -r | grep -v "\s*1 "
   3 wait for GC cycle
   2 semacquire
   2 GC sweep wait

@rsc
Copy link
Contributor

rsc commented Mar 26, 2018

Leaving this for @aclements to close.

@aclements
Copy link
Member

3 wait for GC cycle

Probably these should actually get factored into a single function. It's three copies of very similar code.

2 semacquire

These could be distinct. One is for acquiring semaphores, the other is for sync.Cond.Wait.

2 GC sweep wait

These really are the same thing, but the control flow makes it a little tricky to bring them together, so this is fine.

@gopherbot
Copy link

Change https://golang.org/cl/102605 mentions this issue: runtime: factor waiting on mark phase

@gopherbot
Copy link

Change https://golang.org/cl/102606 mentions this issue: runtime: distinguish semaphore wait from sync.Cond.Wait

gopherbot pushed a commit that referenced this issue Apr 6, 2018
@aclements
runtime: distinguish semaphore wait from sync.Cond.Wait
e13a213 
Updates #24362.

Change-Id: Ided1ab31792f05d9d7a86f17c1bcbd9e9b80052c
Reviewed-on: https://go-review.googlesource.com/102606
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Apr 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
4 participants
@josharian @rsc @aclements @gopherbot