atomic.CompareAndSwap slower than mutexes

[claygod]

To "lock / free", I tried to use "atomic.CompareAndSwap" and "mutex". To my surprise, in a non-parallel use of "mutex" I was faster. Explain why "mutex" faster, because he uses "atomic.CompareAndSwap"?

---

• What version of Go are you using (go version)? 1.7
• What operating system and processor architecture are you using (go env)? Win7/i3

[ALTree]

You should post the code of your benchmarks, to help others understand and/or reproduce the issue.

[claygod]

Code:

package queue

import (
    "sync"
    "sync/atomic"
)

const sizeSection int = 10000

type Message struct {
    id int
}

type Queue struct {
    mu   *sync.Mutex
    db   []Message
    hasp int32
    tail int
    size int
}

func New() Queue {
    q := Queue{
        new(sync.Mutex), make([]Message, sizeSection, sizeSection), 0, 0, sizeSection,
    }
    return q
}

func (q Queue) PushTailCAS(n Message) {
    for {
        if atomic.CompareAndSwapInt32(&q.hasp, 0, 1) {
            break
        }
    }
    q.db[q.tail] = n
    q.tail++
    if q.tail > q.size {
        q.db = append(q.db, make([]Message, sizeSection, sizeSection)...)
        q.size += sizeSection
    }
    atomic.StoreInt32(&q.hasp, 0)
}
func (q Queue) PushTailMutex(n Message) {
    q.mu.Lock()
    q.db[q.tail] = n
    q.tail++
    if q.tail > q.size {
        q.db = append(q.db, make([]Message, sizeSection, sizeSection)...)
        q.size += sizeSection
    }
    q.mu.Unlock()
}

Test:

package queue

import (
    "testing"
)

func BenchmarkPushTailCAS(b *testing.B) {
    b.StopTimer()
    q := New()
    b.StartTimer()
    for i := 0; i < b.N; i++ {
        q.PushTailCAS(Message{id: i})
    }
}

func BenchmarkPushTailMutex(b *testing.B) {
    b.StopTimer()
    q := New()
    b.StartTimer()
    for i := 0; i < b.N; i++ {
        q.PushTailMutex(Message{id: i})
    }
}

Result:

BenchmarkPushTailCAS-4      20000000            77.2 ns/op
BenchmarkPushTailMutex-4    50000000            28.4 ns/op

[Ganners]

You're using a value receiver which means you're copying the Queue.

[dAdAbird]

Yeah, the point in the receiver.

func (q *Queue) PushTailCAS(n Message) {
    for {
        if q.hasp == 0 && atomic.CompareAndSwapInt32(&q.hasp, 0, 1) {
            break
        }
    }
    q.db[q.tail] = n
    q.tail++
    if q.tail == q.size {
        q.db = append(q.db, make([]Message, sizeSection, sizeSection)...)
        q.size += sizeSection
    }
    q.hasp = 0
}

func (q *Queue) PushTailMutex(n Message) {
    q.mu.Lock()
    q.db[q.tail] = n
    q.tail++
    if q.tail == q.size {
        q.db = append(q.db, make([]Message, sizeSection, sizeSection)...)
        q.size += sizeSection
    }
    q.mu.Unlock()
}

With this code I had:

BenchmarkPushTailCAS-8          50000000                25.6 ns/op            48 B/op          0 allocs/op                  
BenchmarkPushTailMutex-8        50000000                32.3 ns/op            48 B/op          0 allocs/op                  

Anyway in the variant with value receiver message always will be pushed in the first element of db

[claygod]

Ganners, I have to remove the blocks of interest

if q.tail > q.size {
        q.db = append(q.db, make([]Message, sizeSection, sizeSection)...)
        q.size += sizeSection
    }

and constant change

const sizeSection int = 1000000000

but it did not change the situation.

[dAdAbird]

Also I don't understand why for

func (q Queue) PushTailCAS(n Message) {
    for {
        if atomic.CompareAndSwapInt32(&q.hasp, 0, 1) {
            break
        }
    }
    q.db[q.tail] = n
    q.tail++
    if q.tail > q.size {
        q.db = append(q.db, make([]Message, sizeSection, sizeSection)...)
        q.size += sizeSection
    }
    atomic.StoreInt32(&q.hasp, 0)
}

compiler allocates a new object and creates write barrier:

00103 (queue.go:29) LEAQ    type."".Queue(SB), AX
00104 (queue.go:29) MOVQ    AX, (SP)
00105 (queue.go:29) CALL runtime.newobject(SB)
00106 (queue.go:29) MOVQ    8(SP), AX
00107 (queue.go:29) MOVQ    AX, "".&q(SP)
00108 (queue.go:29) TESTB   AX, (AX)
00109 (queue.go:29) MOVL runtime.writeBarrier(SB), CX
00110 (queue.go:29) TESTB   CX, CX
00111 (queue.go:29) JNE $0, 218
00112 (queue.go:29) MOVQ    "".q(FP), CX
00113 (queue.go:29) MOVQ    CX, (AX)
00114 (queue.go:29) LEAQ    8(AX), DI
00115 (queue.go:29) LEAQ    "".q+8(FP), SI
00116 (queue.go:29) DUFFCOPY    $854

but for func (q Queue) PushTailMutex(n Message) { - doesn't.

I think it causes the main difference in performance

[claygod]

Ganners, and if that's between the two rows of someone will increase q.tail?

tail: = atomic.AddUint64 (& q.tail, 1)
// Someone here has increased q.tail
size: = atomic.LoadUint64 (& q.size)

Code CompareAndSwap should be protect db

[claygod]

dAdAbird, many thanks, It turns out the matter in a row

(q Queue)

enough to change

(q *Queue)

but I did not expect such an effect!

[dAdAbird]

Yeah, me too!

May be @randall77 @dr2chase can clarify this #17604 (comment)

[randall77]

The compiler moves q to the heap because atomic.CompareAndSwapInt32 is implemented in assembly. Escape analysis has to conservatively assume that any pointer passed to assembly escapes.

We could mark atomic.CompareAndSwapInt32 as noescape, but there's really no point. If you're calling atomic.CompareAndSwapInt32 on something on the stack, you're doing something wrong. There's no need for atomic operations on stack variables.

You definitely want to operate on *Queue, not Queue. If you're copying your synchronization variables and your mutexes, you've got bigger problems than performance.

[extemporalgenome]

It's not copying the mutex.

On Wed, Oct 26, 2016, 8:28 AM Mark Gannaway notifications@github.com
wrote:

The code provided isn't a fare comparison, the PushTailCAS is doing more
work. You're also using a value receiver which means you're copying the
Queue, and therefore also the mutex.

Perhaps something more like this would lead to a fairer comparison:

func (q *Queue) PushTailCAS(n Message) {
tail := atomic.AddUint64(&q.tail, 1)
size := atomic.LoadUint64(&q.size)
if tail >= size {
atomic.AddUint64(&q.size, tail+size)
}
}
func (q *Queue) PushTailMutex(n Message) {
q.mu.Lock()
q.tail++
if q.tail >= q.size {
q.size += sizeSection
}
q.mu.Unlock()
}

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#17604 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAgwpMzi_bdiIJOC5F2t11YDqWY_wmf5ks5q32OpgaJpZM4KhBkc
.

[dAdAbird]

@randall77 Thank you for the explanation! Now it's clear why q escapes.

[randall77]

True. But it is copying hasp which is just as bad.