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

runtime: make all waitReasons distinct? #24362

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

Comments

Projects
None yet
4 participants
@josharian
Copy link
Contributor

commented Mar 12, 2018

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 this to the Go1.11 milestone Mar 12, 2018

@aclements

This comment has been minimized.

Copy link
Member

commented Mar 15, 2018

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

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2018

$ 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

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2018

Leaving this for @aclements to close.

@aclements

This comment has been minimized.

Copy link
Member

commented Mar 26, 2018

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

This comment has been minimized.

Copy link

commented Mar 26, 2018

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

@gopherbot

This comment has been minimized.

Copy link

commented Mar 26, 2018

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

runtime: distinguish semaphore wait from sync.Cond.Wait
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>

@gopherbot gopherbot closed this in fcb7488 Apr 6, 2018

@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.
You can’t perform that action at this time.