Proposal Details
Can we set the parameter spinning
of the runtime.startm
function to true by default to make full use of spinning ms to run tasks and avoid additional thread wake up, such as runtime.injectglist
and runtime.handoffp
?
Comment From: ianlancetaylor
CC @golang/runtime
Comment From: mknyszek
This is a change that may be possible to make, but is not that simple to do correctly. Passing spinning=true
can only happen if nmspinning
is incremented, but just incrementing nmspinning
in all of the places startm
is called is probably wrong.
What are you concretely trying to achieve? Do you have a benchmark or application that you think would benefit? Why would it benefit? Without this information, I don't think we'll be able to move forward here. You could also try implementing it yourself and showing that it is an improvement on some benchmark or application, that would also help this issue move forward. Thanks.
Comment From: jayantxie
@mknyszek I'm not quite sure why runtime.wakep
function sets spinning
to true by default, while other cases such as runtime.injectglist
do not. I have a conjecture that if the existence of spinning ms in Go code is very common, can we also change cases like runtime.injectglist
to spinning, so as to make better use of these spinning ms and reduce unnecessary wakeup.
I tried to simulate such a scenario: a Go program receives n tasks through the network at regular intervals. At the same time, each task starts two goroutines to run. In this case, we have a large number of spinning ms created by runtime.newproc
function, and due to the relatively large number of network tasks, netpoll will also call runtime.injectglist
to create additional non-spinning threads. In this case, these non-spinning threads cannot fully utilize spinning threads to avoid starting, thus increasing additional overhead.
The following is the code to simulate this scenario: https://github.com/jayantxie/spinningm
Run command: ./run.sh
, and then modify $GOROOT/src/runtime/proc.go:
func injectglist(glist *gList) {
// ...
startIdle := func(n int) {
for i := 0; i < n; i++ {
wakep()
}
}
// ...
}
run ./run.sh
again.
And the result is,
- injectglist without spinning:
- injectglist with spinning:
Although this is only a special case and setting "spinning" by default may consume additional CPU when the system is very idle, we can still carefully evaluate this change because spinning calls like runtime.wakep
in Go are very common.
Comment From: jayantxie
@mknyszek Is there any advice?
Comment From: mknyszek
Interesting, I see what you're saying. Off the top of my head, I don't see why wakep
and being conservative about spinning Ms would be a problem here, and may be worth trying. It's also indeed a bit odd that startIdle
actually looks so much like wakep
. Though, it seems like the intent of this code is to get enough threads running to handle everything that just came out of the netpoller, and wakep
is far more conservative than that: if there are any spinning Ms, it will back out. Taking this a step further, perhaps this should only start up max(npidle, n - nmspinning)
threads (wakep
that many times).
My concern, however, is that without an example of a real-world application dealing with this, that we'll just be making a change for the sake of making a change, without a strong sense of how it'll help Go users. Have you seen this happen in a production environment?
CC @prattmic
Comment From: mknyszek
54622 is also related to the CPU cost of extraneous Ms spinning for work.
Comment From: jayantxie
@mknyszek I found this problem in a self-developed network library. In the library I developed, I started a thread by newm
but without a P. It will call a special network poller in a loop to obtain events , and then call injectglist
to wake up the goroutines associated with these events. After testing, I found that if it is changed to spinning=true
, the CPU usage under a fixed QPS can be significantly reduced. Therefore, I simulated a similar scenario with both injectglist
and newproc
calls to indirectly verify this phenomenon.