5 ms·

This is interesting, actually. As the language is specified, it's not wrong to just ignore that add completely -- the variable isn't used inside the goroutine,

This is interesting, actually. As the language is specified, it's not wrong to just ignore that add completely -- the variable isn't used inside the goroutine, and you're not guaranteed a goroutine will run at any particular time. So, code executed right after that routine is spawned will be hitting the 'old' value anyway; hence Dvyukov's comments that this is arguably to spec, or within spec. Think if you actually wrote this code; you'd have some weird behavior if the compiler did what you 'expect' - a couple outputs of low integers then some high ones as the goroutine gets scheduled in. So it's almost certainly just 'wrong' to write something like this, under most (all?) circumstances. Because sync.Wait() isn't used, there's no guarantee the goroutine will execute at all before program halt. That said, I bet there's some related more reasonable scenario where you could argue the compiler should at least do the adding. I'm not sure why atomic.Add* should get special treatment though, and it looks like the fix is to remove some escape-analysis type code, so I'm curious how this will get fixed.

fmstephe 10y ago
Unfortunately this miscompilation probably is within spec, given that the atomic package has no documented ordering. But it's coming. https://github.com/golang/go/issues/5045 https://github.com/golang/go/issues/5045
- gpderetta 10y ago
  This is issue is not related to ordering, but to visibility (i.e., when is a memory side effect globally visible). Many concurrent memory models (C++ at least, java very likely) allow coalescing stores to atomic variables as long as there is no other interleaving side effect; this is normally a desirable optimization. As described by dvyukov, the go compiler seems to be aggressively coalescing all stores till the end of the loop (which never happens). Store coalescing is desirable, but it is hard to specify completely, for example the C++ standard has non normative wording stating that stores should be visible in a reasonable amount of time exactly to prevent this sort of issues.
  - twic 10y ago
    Java only requires that the writes can't be seen out of order - if a thread writes A then B, another thread can't read the new value of B and then the old value of A. If coalescing them makes them all happen atomically, that's fine.
alecrn 10y ago
Yeah, this particular instance seems ok to me. This one makes the example feel weirder: func main() { runtime.GOMAXPROCS(runtime.NumCPU()) fmt.Println(runtime.NumCPU(), runtime.GOMAXPROCS(0)) started := make(chan bool) go func() { started <- true for { atomic.AddUint64(&a, uint64(1)) } }() <-started for { fmt.Println(atomic.LoadUint64(&a)) time.Sleep(time.Second) } } Here we explicitly wait until the goroutine is started, so we know it's scheduled by the time our other loop runs. Here on my computer with go1.8 linux/amd64 it still optimizes out the while loop, which makes sense as nothing changed that would convince the optimizer the loop should remain given the compiler's current logic. If you add time.Sleep(time.Millisecond) to the goroutine loop or any other synchronization it works fine. I'm having trouble thinking of a real world example where you'd want an atomic operation going ham in a loop without any sort of time or synchronization. At the very least a channel indicating when the loop is done would cause the loop to compile.
- the8472 10y ago
  > I'm having trouble thinking of a real world example where you'd want an atomic operation going ham in a loop without any sort of time or synchronization. By using another atomic op as synchronization? After all that's their stated purpose.
  - alecrn 10y ago
    But I don't think in go atomic ops are considered synchronization in the sense that they force two goroutines to synchronize at a particular point like a chan. I.e. a chan send in one goroutine must be matched with a chan receive in another (unless they're buffered). If you have an atomic operation between two synchronization points I'd expect the only guarantee is that it occurs between the two points, and when it does it happens atomically.
- flik 10y ago
  >>Strictly saying this is confirming behavior as we don't give any guarantees about scheduler (just like any other language, e.g. C++). This can be explained as "the goroutine is just not scheduled". Not a GO developer. On a multi-processor machine, how is this a conforming behavior? Is "scheduler cannot give any guarantees" acceptable?
eternalban 10y ago
What "'old' value" are you talking about? This is a pointer: val := atomic.LoadUint64(&a) > Because sync.Wait() isn't used, there's no guarantee the goroutine will execute at all before program halt. It's an infinite loop ... > I'm not sure why atomic.Add* should get special treatment though Because it side effects and arguably it is a form (mem) I/O in context of modern systems.
- 010a 10y ago
  > It's an infinite loop ... In Go, if runtime.GOMAXPROCS() returns 1 or is set to 1 (meaning, Go only has access to a single system thread), the 'for' loop canabalizes the scheduler and the goroutine is rarely or never scheduled. That fact doesn't make the OP's statement true, or have anything to do with the optimization bug, but it is worth pointing out that there are situations where Go correctly cannot guarantee that goroutine is executed.
  - eternalban 10y ago
    Yes, I am aware of that. Just addressing the OP's misread of the code. But do note that /may/ change given the GC considerations: https://github.com/golang/go/issues/10958 https://github.com/golang/go/issues/10958
Filligree 10y ago
> Because sync.Wait() isn't used, there's no guarantee the goroutine will execute at all before program halt. This is the sort of reasoning I'd expect to see from C apologists. Go should be better... There are implicit guarantees on when goroutines are scheduled, too. The scheduler makes some attempts at fairly spreading the CPU.
- knorker 10y ago
  > There are implicit guarantees on when goroutines are scheduled But there's also explicitly no guarantees on that.
- omginternets 10y ago
  This is unfair, I think. Go offers strong guarantees that C does not. It has fewer sharp edges than C; it also asserts a principle of not polishing away sharp edges with concurrency. You don't have to do everything better than C to be a huge improvement over C.
kevhito 10y ago
I don't understand. By your argument, and some of the replies, because "you're not guaranteed a goroutine will run at any particular time", the compiler is free to elide any suffix of a goroutine's code, even if that code has visible side effects. I think there is something more specific to atomic going on here, where the same kinds of optimizations are already forbidden for other kinds of visible side effects like printing.
- omginternets 10y ago
  >even if that code has visible side effects The argument is that you told it to do that. "Just print whenever", and whenever can mean never if the world dies prematurely.
  - kevhito 10y ago
    So you are telling me a valid optimization for Go is to elide the entire contents of all goroutines? And by that argument, since main() itself is an implicitly-called goroutine (so far as I know), then the compiler can elide the entire contents of main() too, and then the rest of the contents of the program. </reductio-ad-absurdum>
    - NateDad 10y ago
      Well, sort of, yes. https://play.golang.org/p/qyfqZA5pww https://play.golang.org/p/qyfqZA5pww func main() { go fmt.Println("Hi!") } This is a valid go program (eliding import etc), and it'll almost never print anything. This is actually a classic mistake when starting with goroutines - not adding synchronization to make sure your main goroutine doesn't exit before any spawned goroutines.
    - CasualSuperman 10y ago
      Yes, but then the program would never terminate, because that's what happens at the end of main. If main returns, it's saying that the rest of the code in main has executed.

fmstephe 10y ago

Unfortunately this miscompilation probably is within spec, given that the atomic package has no documented ordering. But it's coming. https://github.com/golang/go/issues/5045 https://github.com/golang/go/issues/5045

gpderetta 10y ago
This is issue is not related to ordering, but to visibility (i.e., when is a memory side effect globally visible). Many concurrent memory models (C++ at least, java very likely) allow coalescing stores to atomic variables as long as there is no other interleaving side effect; this is normally a desirable optimization. As described by dvyukov, the go compiler seems to be aggressively coalescing all stores till the end of the loop (which never happens). Store coalescing is desirable, but it is hard to specify completely, for example the C++ standard has non normative wording stating that stores should be visible in a reasonable amount of time exactly to prevent this sort of issues.
- twic 10y ago
  Java only requires that the writes can't be seen out of order - if a thread writes A then B, another thread can't read the new value of B and then the old value of A. If coalescing them makes them all happen atomically, that's fine.

alecrn 10y ago

Yeah, this particular instance seems ok to me. This one makes the example feel weirder: func main() { runtime.GOMAXPROCS(runtime.NumCPU()) fmt.Println(runtime.NumCPU(), runtime.GOMAXPROCS(0)) started := make(chan bool) go func() { started <- true for { atomic.AddUint64(&a, uint64(1)) } }() <-started for { fmt.Println(atomic.LoadUint64(&a)) time.Sleep(time.Second) } } Here we explicitly wait until the goroutine is started, so we know it's scheduled by the time our other loop runs. Here on my computer with go1.8 linux/amd64 it still optimizes out the while loop, which makes sense as nothing changed that would convince the optimizer the loop should remain given the compiler's current logic. If you add time.Sleep(time.Millisecond) to the goroutine loop or any other synchronization it works fine. I'm having trouble thinking of a real world example where you'd want an atomic operation going ham in a loop without any sort of time or synchronization. At the very least a channel indicating when the loop is done would cause the loop to compile.

the8472 10y ago
> I'm having trouble thinking of a real world example where you'd want an atomic operation going ham in a loop without any sort of time or synchronization. By using another atomic op as synchronization? After all that's their stated purpose.
- alecrn 10y ago
  But I don't think in go atomic ops are considered synchronization in the sense that they force two goroutines to synchronize at a particular point like a chan. I.e. a chan send in one goroutine must be matched with a chan receive in another (unless they're buffered). If you have an atomic operation between two synchronization points I'd expect the only guarantee is that it occurs between the two points, and when it does it happens atomically.
flik 10y ago
>>Strictly saying this is confirming behavior as we don't give any guarantees about scheduler (just like any other language, e.g. C++). This can be explained as "the goroutine is just not scheduled". Not a GO developer. On a multi-processor machine, how is this a conforming behavior? Is "scheduler cannot give any guarantees" acceptable?

eternalban 10y ago

What "'old' value" are you talking about? This is a pointer: val := atomic.LoadUint64(&a) > Because sync.Wait() isn't used, there's no guarantee the goroutine will execute at all before program halt. It's an infinite loop ... > I'm not sure why atomic.Add* should get special treatment though Because it side effects and arguably it is a form (mem) I/O in context of modern systems.

010a 10y ago
> It's an infinite loop ... In Go, if runtime.GOMAXPROCS() returns 1 or is set to 1 (meaning, Go only has access to a single system thread), the 'for' loop canabalizes the scheduler and the goroutine is rarely or never scheduled. That fact doesn't make the OP's statement true, or have anything to do with the optimization bug, but it is worth pointing out that there are situations where Go correctly cannot guarantee that goroutine is executed.
- eternalban 10y ago
  Yes, I am aware of that. Just addressing the OP's misread of the code. But do note that /may/ change given the GC considerations: https://github.com/golang/go/issues/10958 https://github.com/golang/go/issues/10958

Filligree 10y ago

> Because sync.Wait() isn't used, there's no guarantee the goroutine will execute at all before program halt. This is the sort of reasoning I'd expect to see from C apologists. Go should be better... There are implicit guarantees on when goroutines are scheduled, too. The scheduler makes some attempts at fairly spreading the CPU.

knorker 10y ago
> There are implicit guarantees on when goroutines are scheduled But there's also explicitly no guarantees on that.
omginternets 10y ago
This is unfair, I think. Go offers strong guarantees that C does not. It has fewer sharp edges than C; it also asserts a principle of not polishing away sharp edges with concurrency. You don't have to do everything better than C to be a huge improvement over C.

kevhito 10y ago

I don't understand. By your argument, and some of the replies, because "you're not guaranteed a goroutine will run at any particular time", the compiler is free to elide any suffix of a goroutine's code, even if that code has visible side effects. I think there is something more specific to atomic going on here, where the same kinds of optimizations are already forbidden for other kinds of visible side effects like printing.

omginternets 10y ago
>even if that code has visible side effects The argument is that you told it to do that. "Just print whenever", and whenever can mean never if the world dies prematurely.
- kevhito 10y ago
  So you are telling me a valid optimization for Go is to elide the entire contents of all goroutines? And by that argument, since main() itself is an implicitly-called goroutine (so far as I know), then the compiler can elide the entire contents of main() too, and then the rest of the contents of the program. </reductio-ad-absurdum>
  - NateDad 10y ago
    Well, sort of, yes. https://play.golang.org/p/qyfqZA5pww https://play.golang.org/p/qyfqZA5pww func main() { go fmt.Println("Hi!") } This is a valid go program (eliding import etc), and it'll almost never print anything. This is actually a classic mistake when starting with goroutines - not adding synchronization to make sure your main goroutine doesn't exit before any spawned goroutines.
  - CasualSuperman 10y ago
    Yes, but then the program would never terminate, because that's what happens at the end of main. If main returns, it's saying that the rest of the code in main has executed.