Consider moving from sync.RWMutex to sync.Mutex

[Jacalz]

Checklist

• I have searched the issue tracker for open issues that relate to the same feature, before opening a new one.
• This issue only relates to a single feature. I will open new issues for any other features.

Is your feature request related to a problem?

We rely quite heavily on sync.RWMutex within the project and it might not always be the magical performance improvement compared to sync.Mutex that we might sometimes think it is. Most notably, there is the issue of golang/go#17973. The RWMutex type is also larger so takes up more space in the cache line (golang/go#17973 (comment)) plus it is more complicated internally so acquiring locks is slower that Mutex in cases where there is no lock contention.

As part of benchmarking #4509 (comment), I got the following numbers when testing atomic vs mutexes with no lock contention. Take it with a little grain of salt given that the idea with RWMutex is to be faster for simultaneous accesses (read/write) and this test is only considering single-threaded usage:

goos: linux
goarch: amd64
pkg: fyne.io/fyne/v2/internal/async
cpu: Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz
BenchmarkAtomicLoadAndStore-8    	              408362744	         2.538 ns/op	       0 B/op	       0 allocs/op
BenchmarkMutexLoadAndStore-8     	              61530513	        18.61 ns/op	       0 B/op	       0 allocs/op
BenchmarkRWMutexLoadAndStoreWithRLockForLoad-8   	       34136662	        29.97 ns/op	       0 B/op	       0 allocs/op
BenchmarkRWMutexLoadAndStoreWithLockForLoad-8   	26073561	        42.25 ns/op	       0 B/op	       0 allocs/op

Is it possible to construct a solution with the existing API?

Yes. We can evaluate using Mutex in some cases and maybe even switching to sync.Map or sync/atomic in some cases.

Describe the solution you'd like to see.

We need to benchmark our use cases and see if sync.RWMutex is helping us or hurting us in terms of performance.

[dweymouth]

Worth noting that for sync.Map, the official documentation advises that most code should use a regular map protected by a mutex, and only for specific use patterns is sync.Map better

[Bluebugs]

First, I don't think fyne application are seriously impacted by the big reported against go. How many desktop currently have 16 or more core.

Now the reasoning to use rwmutex is to allow reader to have fast and unblocked access. Most case in a ui should be read with little write. If we have heavy writer case, then that's something to look into first and see if we could switch more to read.

[Jacalz]

Worth noting that for sync.Map, the official documentation advises that most code should use a regular map protected by a mutex, and only for specific use patterns is sync.Map better

Indeed. It does however document a few cases where it is better (mostly reads or caches that only grow, I think). We do have a few uses of it already It might be a case that some of those should just use a mutex though.

[Jacalz]

First, I don't think fyne application are seriously impacted by the big reported against go. How many desktop currently have 16 or more core.

Absolutely. It is worth noting that I have gotten the understanding from Gophers Slack (#performance etc.) that RWMutex generally is discouraged in favour of a regular Mutex
But don't quite me on that :)

[Bluebugs]

Yeah, #performance has usually a very different profile usage than fyne applications. I wish I had 192 core cpu :-D

[Jacalz]

I don't think it was about the issue about scaling but more of a general idea that Mutex might be better and faster in many other situations. My benchmarking does show that for the same 1:1 locking it can be considerably slower when there is no contention.