-2

In go standard package src/sync/once.go, a recent revision change the snippets

if atomic.LoadUint32(&o.done) == 1 {
        return
    }
//otherwise
...

to:

//if atomic.LoadUint32(&o.done) == 1 {
//      return
//  }
if atomic.LoadUint32(&o.done) == 0 {
        ...
    }

the question is, according to this change, hot path is no longer explicit in the code, does this change has bad impact on branch prediction ? does go compiler make some help in the subsequent run of this function or the whole thing of branch prediction is on cpu?

commit page:https://github.com/golang/go/commit/ca8354843ef9f30207efd0a40bb6c53e7ba86892

Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
agnes
  • 11
  • 1
  • 1
    Could you provide details like code version of this "a recent revision change", for others to check? – rustyhu Aug 30 '22 at 02:24
  • 2
    It appears that [this](https://github.com/golang/go/commit/ca8354843ef9f30207efd0a40bb6c53e7ba86892) is the commit you are referring to (three years ago so not that recent!). The comment mentions inlining but I see no mention of branch prediction? – Brits Aug 30 '22 at 02:26
  • On most ISAs, there are no explicit branch hints in assembly language, so the compiler can't hint even if it wanted to. However, laying out the hot path through a function so it's contiguous can be a win, not exactly because of branch *prediction*, as in [this example](https://stackoverflow.com/questions/7346929/what-is-the-advantage-of-gccs-builtin-expect-in-if-else-statements/31540623#31540623). Related: [Is there a compiler hint for GCC to force branch prediction to always go a certain way?](https://stackoverflow.com/a/30136196) – Peter Cordes Aug 30 '22 at 02:40
  • @PeterCordes Can I understand that it is the responsibility of the CPU，that I delete the if done==1 branch in my code and I trust the CPU can handle this? – agnes Aug 30 '22 at 03:20
  • If such questions come up in the future: Ask them on the golang-nuts mailing list. – Volker Aug 30 '22 at 05:08

1 Answers1

1

The particular commit you're talking about (found by Brits in a comment) is not an attempt to make use of branch prediction. It's using knowledge about how the Go compiler does inline expansion of small functions.

We're given the option of writing a function in this way:

func (o *Object) Operate() {
    if (o.alreadyDone) { return }
    ... some code ...
}

Or, alternatively, writing it this way:

func (o *Object) Operate() {
    if (!o.alreadyDone) { o.reallyOperate() }
}

where o.reallyOperate takes over the ... some code ... part.

If the some code part is more than a few instructions long and is written the way the original once.Do was, the Go compiler implements the function by having the caller call the actual function. But when it's as short as the replacement, the caller implements the function as an inline test, branch, and then maybe call the reallyOperate function.

Since sync.Once actually calls the function only once per Once object, and the rest of the time, does not call, this inline expansion results in not making the call on every Do call except the first one. This actually makes the code at the call site bigger (by one or two instructions) but since the call is normally not executed, the result is normally faster.

torek
  • 448,244
  • 59
  • 642
  • 775
  • So it's manual partial-inlining of just the early-out check from a big function? Yeah, that's a useful optimization in other languages, too, such as C: [How well do compilers (with link-time optimization) cope with functions that return quickly (early-out paths)?](https://stackoverflow.com/q/29706147) – Peter Cordes Aug 30 '22 at 04:12
  • I think I know what the main idea of that commit. The only thing I want to check is that does sentence 'if (o.alreadyDone) { return }' exist or not make any difference. If not, why the previous version has that? If that does has some effect, what is that and how to handle that? – agnes Aug 30 '22 at 04:28
  • @agnes: it makes a difference *to the Go compiler*, which has a particular, specific strategy for doing function inlining. Other compilers have different strategies and hence may work better with different source code tweaks. – torek Aug 30 '22 at 04:52
  • @torek I have test that with or not 'if (o.alreadyDone) { return }', compiler can inlining despite. Did you get my point? – agnes Aug 30 '22 at 05:08
  • Did you test the version of the Go compiler that was in use at the time they changed the source? – torek Aug 30 '22 at 07:40