Hi all, In this particular case, this is a toy example of course, but for this toy example, it is absolutely a case where the performance of the synchronization primitive literally does not matter at all. (If I followed here, the intent is seemingly to watch a long running task, and the example has a 500ms sleep, etc.).
That said, sometimes performance does matter. If instead this was a DIFFERENT example that was instead in some tight performance critical inner loop, the performance of the synchronization primitive for a stop flag can start to be meaningful (which of course should first be demonstrated by your benchmarking and/or your profiling of your particular program -- "premature optimization is the root of all evil" and all that). Empirically speaking, that is then a situation where I've seen people start to get into discussion around "well on amd64 you can rely on X and Y so we can get away with a stop flag that doesn't use a mutex or an atomic", and then people start talking about benign data races, and then other people start talking about "there are no benign data races", and then there's the reference to Dmitry Vyukov's article on benign data races, etc.: https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong To be honest I've seen it take up a fair amount of energy just to discuss, and again empirically speaking I've seen very smart people make mistakes here. One question I have regarding using an atomic for a stop flag is whether or not there is material performance overhead compared to a simple unprotected/non-atomic stop flag. I looked at that question a bit circa go ~1.7, so possibly stale info here, and I haven't gone back to look at my more detailed notes, but if I recall correctly I think my conclusion at the time was: 1. If your use case is a stop flag that will be checked many times (say, in a tight inner loop), and the stop flag only gets set rarely, then the performance of the set doesn't matter much, the assembly emitted for an atomic load (such as atomic.LoadUint64) seems to be identical for the assembly emitted for a simple load (say, *myUnint64Ptr) based on some spot checking a while ago (with a sample of assembly from 1.9 pasted in at the bottom of this post for comparison purposes). 2. Basic micro benchmarks didn't show a difference between an atomic load and an unprotected load for a stop flag. 3. There might be some theoretical possible overhead due to the go compiler will do instruction reordering in general, but won't do instruction reordering across function calls, so that could be one difference that might or might not make a difference depending on your exact code (though likely modest impact)? Regarding this last point, I'm just an interested spectator when it comes to the go compiler, so just to be clear I don't really know this definitively. At least for us at the time, the quick test program & comparison of assembly was enough to move us past the whole "Well, on amd64 you can get away with X" discussion, so I didn't delve too much deeper than that at the time. In any event, sharing this sample assembly with the community in case anyone is interested and/or has additional commentary on the particulars here in terms of performance impact in go of using an atomic load vs an unprotected stop flag for the (admittedly somewhat rare) cases when the nanoseconds do indeed matter. (And for me, a clean report from the race detector trumps the performance arguments, but that doesn't mean I'm not curious about the performance...). Here is a simple test program: https://play.golang.org/p/PaCQwb5m9ag func simpleLoadUint64(inputPtr *uint64) uint64 { // normal load of *inputPtr return *inputPtr } func atomicLoadUint64(inputPtr *uint64) uint64 { // atomic.LoadUint64 atomically loads *inputPtr return atomic.LoadUint64(inputPtr) } And here is the corresponding assembly snippets (from go 1.9): go build -gcflags -S atomic_vs_normal_load.go // trivial function with an unprotected load "".simpleLoadUint64 STEXT nosplit size=14 args=0x10 locals=0x0 0x0000 00000 (atomic_vs_normal_load.go:8) TEXT "".simpleLoadUint64(SB), NOSPLIT, $0-16 0x0000 00000 (atomic_vs_normal_load.go:8) FUNCDATA $0, gclocals·aef1f7ba6e2630c93a51843d99f5a28a(SB) 0x0000 00000 (atomic_vs_normal_load.go:8) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) 0x0000 00000 (atomic_vs_normal_load.go:8) MOVQ "".inputPtr+8(SP), AX 0x0005 00005 (atomic_vs_normal_load.go:10) MOVQ (AX), AX 0x0008 00008 (atomic_vs_normal_load.go:10) MOVQ AX, "".~r1+16(SP) 0x000d 00013 (atomic_vs_normal_load.go:10) RET 0x0000 48 8b 44 24 08 48 8b 00 48 89 44 24 10 c3 H.D$.H..H.D$.. // trivial function with a sync/atomic LoadUint64: "".atomicLoadUint64 STEXT nosplit size=14 args=0x10 locals=0x0 0x0000 00000 (atomic_vs_normal_load.go:13) TEXT "".atomicLoadUint64(SB), NOSPLIT, $0-16 0x0000 00000 (atomic_vs_normal_load.go:13) FUNCDATA $0, gclocals·aef1f7ba6e2630c93a51843d99f5a28a(SB) 0x0000 00000 (atomic_vs_normal_load.go:13) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) 0x0000 00000 (atomic_vs_normal_load.go:13) MOVQ "".inputPtr+8(SP), AX 0x0005 00005 (atomic_vs_normal_load.go:15) MOVQ (AX), AX 0x0008 00008 (atomic_vs_normal_load.go:15) MOVQ AX, "".~r1+16(SP) 0x000d 00013 (atomic_vs_normal_load.go:15) RET 0x0000 48 8b 44 24 08 48 8b 00 48 89 44 24 10 c3 H.D$.H..H.D$.. --thepudds On Saturday, March 17, 2018 at 10:37:48 AM UTC-4, matthe...@gmail.com wrote: > > I think the second example alternative given (playground link above) has a >> data race? > > > I’m not surprised that the race detector sees something (a read can happen > during a write of the checked bool) but I don’t think this could actually > cause problems because the var’s memory value will always be 0 or 1. > > There may be implementation details or future implementation details that > cause a problem though, so one option could be to protect the bool as a > shared resource with a mutex or equivalent, but I think rog’s solution is > better anyway (the first one). > > They all involve either repeatedly checking on a timer or checking the >> value of another field (like polling) to see whether the long running task >> should be stopped. > > > Right, now every iteration of the loop has more work added. > > Using os.Cmd may be an option, where you can call Kill on the separate > process. > > Matt > > On Saturday, March 17, 2018 at 8:01:33 AM UTC-5, thepud...@gmail.com > wrote: >> >> *> "Here's another way: https://play.golang.org/p/gEDef3LolAZ >>> <https://play.golang.org/p/gEDef3LolAZ> "* >> >> >> Hi all, >> >> I think the second example alternative given (playground link above) has >> a data race? >> >> Sample race detector run just now. (The two reports are inverses of each >> other: read then write vs. write then read). >> >> ----------------------------------------------------------------------- >> go run -race stop_flag_from_gonuts.go >> >> . . . . . quit sending ... >> after quit sent================== >> . >> WARNING: DATA RACE >> Write at 0x00c042072000 by goroutine 8: >> main.f.func1() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:13 +0x59 >> >> Previous read at 0x00c042072000 by goroutine 6: >> main.f() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:17 +0x102 >> >> Goroutine 8 (running) created at: >> main.f() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:11 +0x8a >> >> Goroutine 6 (running) created at: >> main.main() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:28 +0x70 >> ================== >> ================== >> WARNING: DATA RACE >> Read at 0x00c042072000 by goroutine 6: >> main.f() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:17 +0x102 >> >> Previous write at 0x00c042072000 by goroutine 8: >> main.f.func1() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:13 +0x59 >> >> Goroutine 6 (running) created at: >> main.main() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:28 +0x70 >> >> Goroutine 8 (finished) created at: >> main.f() >> C:/cygwin64/bin/gonuts/stop_flag_from_gonuts.go:11 +0x8a >> ================== >> quit called >> Found 2 data race(s) >> exit status 66 >> ----------------------------------------------------------------------- >> >> --thepudds >> >> On Friday, March 16, 2018 at 11:04:38 AM UTC-4, matthe...@gmail.com >> wrote: >>> >>> While this is running, your select won't be receiving on the quit >>>> channel, even if it is non-nil. >>>> If you want to be able to cancel it, you'll need to make the code in >>>> the loop responsive to the quit channel >>>> (for example, by using a select like you're using in f already). >>> >>> >>> The default select case does it: https://play.golang.org/p/jlfaXu6TZ8L >>> >>> Here's another way: https://play.golang.org/p/gEDef3LolAZ >>> >>> Matt >>> >>> On Friday, March 16, 2018 at 9:45:00 AM UTC-5, Sathish VJ wrote: >>>> >>>> All the examples I've seen use some kind of ticker to run various cases >>>> of a select statement. But how does one run a long running task that is >>>> still cancelable? >>>> >>>> >>>> In the example below the quit part is never reached. >>>> >>>> https://play.golang.org/p/PLGwrUvKaqn (it does not run properly on >>>> play.golang.org). >>>> >>>> package main >>>> >>>> >>>> import ( >>>> "fmt" >>>> "os" >>>> "time" >>>> ) >>>> >>>> >>>> func f(quit chan bool) { >>>> for { >>>> select { >>>> case <-time.After(0 * time.Second): >>>> // start long running task immediately. >>>> for { >>>> time.Sleep(500 * time.Millisecond) >>>> fmt.Printf(". ") >>>> } >>>> case <-quit: >>>> fmt.Println("quit called") >>>> //deallocate resources in other long running task and then return >>>> from function. >>>> os.Exit(0) // or return >>>> } >>>> } >>>> } >>>> >>>> >>>> func main() { >>>> var quit chan bool >>>> go f(quit) >>>> >>>> >>>> println("quit sending ... ") >>>> quit <- true >>>> println("after quit sent") >>>> >>>> >>>> var i chan int >>>> <-i >>>> } >>>> >>>> >>>> -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
