{"action":"created","issue":{"url":"https://api.github.com/repos/golang/go/issues/10958","repository_url":"https://api.github.com/repos/golang/go","labels_url":"https://api.github.com/repos/golang/go/issues/10958/labels{/name}","comments_url":"https://api.github.com/repos/golang/go/issues/10958/comments","events_url":"https://api.github.com/repos/golang/go/issues/10958/events","html_url":"https://github.com/golang/go/issues/10958","id":81098230,"number":10958,"title":"runtime: tight loops should be preemptible","user":{"login":"aclements","id":2688315,"avatar_url":"https://avatars.githubusercontent.com/u/2688315?v=3","gravatar_id":"","url":"https://api.github.com/users/aclements","html_url":"https://github.com/aclements","followers_url":"https://api.github.com/users/aclements/followers","following_url":"https://api.github.com/users/aclements/following{/other_user}","gists_url":"https://api.github.com/users/aclements/gists{/gist_id}","starred_url":"https://api.github.com/users/aclements/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aclements/subscriptions","organizations_url":"https://api.github.com/users/aclements/orgs","repos_url":"https://api.github.com/users/aclements/repos","events_url":"https://api.github.com/users/aclements/events{/privacy}","received_events_url":"https://api.github.com/users/aclements/received_events","type":"User","site_admin":false},"labels":[{"id":373399998,"url":"https://api.github.com/repos/golang/go/labels/NeedsFix","name":"NeedsFix","color":"ededed","default":false}],"state":"open","locked":false,"assignee":{"login":"randall77","id":6889504,"avatar_url":"https://avatars.githubusercontent.com/u/6889504?v=3","gravatar_id":"","url":"https://api.github.com/users/randall77","html_url":"https://github.com/randall77","followers_url":"https://api.github.com/users/randall77/followers","following_url":"https://api.github.com/users/randall77/following{/other_user}","gists_url":"https://api.github.com/users/randall77/gists{/gist_id}","starred_url":"https://api.github.com/users/randall77/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/randall77/subscriptions","organizations_url":"https://api.github.com/users/randall77/orgs","repos_url":"https://api.github.com/users/randall77/repos","events_url":"https://api.github.com/users/randall77/events{/privacy}","received_events_url":"https://api.github.com/users/randall77/received_events","type":"User","site_admin":false},"assignees":[{"login":"randall77","id":6889504,"avatar_url":"https://avatars.githubusercontent.com/u/6889504?v=3","gravatar_id":"","url":"https://api.github.com/users/randall77","html_url":"https://github.com/randall77","followers_url":"https://api.github.com/users/randall77/followers","following_url":"https://api.github.com/users/randall77/following{/other_user}","gists_url":"https://api.github.com/users/randall77/gists{/gist_id}","starred_url":"https://api.github.com/users/randall77/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/randall77/subscriptions","organizations_url":"https://api.github.com/users/randall77/orgs","repos_url":"https://api.github.com/users/randall77/repos","events_url":"https://api.github.com/users/randall77/events{/privacy}","received_events_url":"https://api.github.com/users/randall77/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/golang/go/milestones/47","html_url":"https://github.com/golang/go/milestones/Go1.9Early","labels_url":"https://api.github.com/repos/golang/go/milestones/47/labels","id":2038341,"number":47,"title":"Go1.9Early","description":"","creator":{"login":"quentinmit","id":115761,"avatar_url":"https://avatars.githubusercontent.com/u/115761?v=3","gravatar_id":"","url":"https://api.github.com/users/quentinmit","html_url":"https://github.com/quentinmit","followers_url":"https://api.github.com/users/quentinmit/followers","following_url":"https://api.github.com/users/quentinmit/following{/other_user}","gists_url":"https://api.github.com/users/quentinmit/gists{/gist_id}","starred_url":"https://api.github.com/users/quentinmit/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/quentinmit/subscriptions","organizations_url":"https://api.github.com/users/quentinmit/orgs","repos_url":"https://api.github.com/users/quentinmit/repos","events_url":"https://api.github.com/users/quentinmit/events{/privacy}","received_events_url":"https://api.github.com/users/quentinmit/received_events","type":"User","site_admin":false},"open_issues":101,"closed_issues":3,"state":"open","created_at":"2016-09-29T18:06:31Z","updated_at":"2016-11-16T15:05:58Z","due_on":"2017-04-30T00:00:00Z","closed_at":null},"comments":32,"created_at":"2015-05-26T19:18:36Z","updated_at":"2016-11-17T19:26:33Z","closed_at":null,"body":"Currently goroutines are only preemptible at function call points. Hence, it's possible to write a tight loop (e.g., a numerical kernel or a spin on an atomic) with no calls or allocation that arbitrarily delays preemption. This can result in arbitrarily long pause times as the GC waits for all goroutines to stop.\n\nIn unusual situations, this can even lead to deadlock when trying to stop the world. For example, the runtime's TestGoroutineParallelism tries to prevent GC during the test to avoid deadlock. It runs several goroutines in tight loops that communicate through a shared atomic variable. If the coordinator that starts these is paused part way through, it will deadlock.\n\nOne possible fix is to insert preemption points on control flow loops that otherwise have no preemption points. Depending on the cost of this check, it may also require unrolling such loops to amortize the overhead of the check.\n\nThis has been a longstanding issue, so I don't think it's necessary to fix it for 1.5. It can cause the 1.5 GC to miss its 10ms STW goal, but code like numerical kernels is probably not latency-sensitive. And as far as I can tell, causing a deadlock like the runtime test can do requires looking for trouble.\n\n@RLH \n"},"comment":{"url":"https://api.github.com/repos/golang/go/issues/comments/261343596","html_url":"https://github.com/golang/go/issues/10958#issuecomment-261343596","issue_url":"https://api.github.com/repos/golang/go/issues/10958","id":261343596,"user":{"login":"aclements","id":2688315,"avatar_url":"https://avatars.githubusercontent.com/u/2688315?v=3","gravatar_id":"","url":"https://api.github.com/users/aclements","html_url":"https://github.com/aclements","followers_url":"https://api.github.com/users/aclements/followers","following_url":"https://api.github.com/users/aclements/following{/other_user}","gists_url":"https://api.github.com/users/aclements/gists{/gist_id}","starred_url":"https://api.github.com/users/aclements/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aclements/subscriptions","organizations_url":"https://api.github.com/users/aclements/orgs","repos_url":"https://api.github.com/users/aclements/repos","events_url":"https://api.github.com/users/aclements/events{/privacy}","received_events_url":"https://api.github.com/users/aclements/received_events","type":"User","site_admin":false},"created_at":"2016-11-17T19:26:33Z","updated_at":"2016-11-17T19:26:33Z","body":"https://go-review.googlesource.com/c/33093/ adds GOEXPERIMENT=preemptibleloops, which enables the compiler to add preemption points to all loops. As of CL 4, this adds preemption points to essentially *all* loops, even if they already have preemption points. The overhead is quite high on some benchmarks, but it's clear that there's also a lot of headroom available to improve this. The current plan is to commit this to Go 1.8 under the GOEXPERIMENT for diagnostic purposes, and to try to reduce the overhead and enable this by default for Go 1.9.\r\n\r\nSome things we can do to improve the overhead (many of which have been discussed above):\r\n\r\n* Improve the efficiency of the check. I haven't looked at the generated code, but we're probably not getting the tightest check possible. In particular, we may be introducing spills into the hot path, rather than sinking them to the preemption path.\r\n\r\n* Don't add preemption points to loops that already have preemption points. This requires knowing which functions are or could be nosplit, which is complicated by the auto-nosplit logic. There's some evidence that auto-nosplit isn't doing us any good anyway, so maybe we just disable that? Unfortunately, this doesn't help with the tightest of loops, where the overhead is the highest.\r\n\r\n* Unroll loops enough to amortize the cost of the check. Unrolling is generally discouraged these days because CPUs are really, really good at running tight loops.\r\n\r\n* Use a register check instead of a memory check. Currently we're checking the stack bound, which requires a memory load. A different approach would be to reserve some register during these loops, have the compiler record the presence of this register in the PCDATA, and set the register by sending a signal to the thread. This would trade a cheaper check for a more costly preemption, but requires mucking with signals, which is always a pain. Relative to @griesemer's suggestion of creating a second copy of the loop, this requires stealing a register, but seems simpler overall to me.\r\n\r\n* Use asynchronous STW (suggested by @cherrymui). At this point, sweep termination and mark termination basically don't need goroutines to be a safe points. We could stop all threads asynchronously using a signal, perform sweep/mark termination, and then resume threads. This doesn't help with stack scanning, which must be done at a safe point, so an unpreemptible loop could still delay mark termination if it ran for long enough. There are some sequences in the runtime that must be \"atomic\" with respect to the garbage collector, but in these it would be easy for the signal to set a flag, let the thread continue, and then check the flag at the end of the sequence."}}
{"actor":{"display_login":"aclements"}}