Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using atomics instead of mutexes could improve performance #120

Open
enkeyz opened this issue Apr 22, 2022 · 8 comments
Open

Using atomics instead of mutexes could improve performance #120

enkeyz opened this issue Apr 22, 2022 · 8 comments

Comments

@enkeyz
Copy link

enkeyz commented Apr 22, 2022

Using atomics instead of mutexes can improve performance(just a small micro optimization). Also made a benchmark, so you can see the difference. I'd make a PR if interested @schollz

package main

import (
	"sync"
	"sync/atomic"
)

type WithMutex struct {
	mu    sync.Mutex
	value int64
}

type WithAtomic struct {
	value int64
}

func (wm *WithMutex) Increase() {
	wm.mu.Lock()
	defer wm.mu.Unlock()
	wm.value++
}

func (wa *WithAtomic) Increase() {
	atomic.AddInt64(&wa.value, 1)
}

benchmark file:

package main

import "testing"

func BenchmarkMutex(b *testing.B) {
	tMutex := WithMutex{}

	for i := 0; i < b.N; i++ {
		tMutex.Increase()
	}
}

func BenchmarkAtomic(b *testing.B) {
	tAtomic := WithAtomic{}

	for i := 0; i < b.N; i++ {
		tAtomic.Increase()
	}
}

benchmark result:

goos: linux
goarch: amd64
pkg: test_atomic
cpu: Intel(R) Core(TM) i5-7600K CPU @ 3.80GHz
BenchmarkMutex
BenchmarkMutex-4        79002190                14.23 ns/op            0 B/op          0 allocs/op
BenchmarkAtomic
BenchmarkAtomic-4       183248221                6.359 ns/op           0 B/op          0 allocs/op
PASS
ok      test_atomic     2.977s
@enkeyz enkeyz changed the title Using atomic instead of mutexes could improve performance Using atomics instead of mutexes could improve performance Apr 22, 2022
@schollz
Copy link
Owner

schollz commented Apr 22, 2022

Would love a PR ! Looks awesome

@rustiever
Copy link

hi @schollz, can i take up this ?

@schollz
Copy link
Owner

schollz commented Oct 5, 2022

@rustiever Sure

@rustiever
Copy link

Hey it looks like atomic package doesn't support atomic operation on float64, and are we trying to remove mutex completely? @schollz

@schollz
Copy link
Owner

schollz commented Oct 6, 2022

I think the goal of this PR is for optimization, so if mutex isn't optimal yes

@rustiever
Copy link

do you have any idea how to support atomic operations on float64?

@martikan
Copy link

@rustiever

Maybe its an old thread. But you can use it with math.Float64frombits() and math.Float64bits()..
Transform an unsigned integer to float and you can do the opposite as well.
After that you can use the atomic operations with unsigned int.
This 'trick' works because int32/64 and float32/64 are stored in the same data structure.

@mxey
Copy link

mxey commented May 7, 2024

I would recommend against this. It would make the code harder to understand, and I doubt the locking is actually a bottleneck. Sending out the rendered progress bar is probably the most expensive part, but a profile should tell you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants