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

Jump to bottom

sync: RWMutex.RLock is 2x slower than Mutex.Lock #38813

Closed
gaurav1086 opened this issue May 2, 2020 · 7 comments
Closed

sync: RWMutex.RLock is 2x slower than Mutex.Lock #38813

gaurav1086 opened this issue May 2, 2020 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@gaurav1086
Copy link
Contributor

What version of Go are you using (go version)?

go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/gaurav/.cache/go-build"
GOENV="/home/gaurav/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/gaurav/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.14"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.14/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build924554981=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Ran a benchmark with sync.Mutex vs sync.RWMutex and found the mutex.Lock() to be 2x faster than the RWMutex.RLock()

Description: Two scenarios (with lock and rwlock) with 1000 goroutines contending to read data from a slice (size = 10) and write to independent files.

https://play.golang.com/p/RZ-bE3OJ4H5

Output: Lock vs RWLocks() in seconds
Locks RWLocks
2.589610687s 5.52346426s
2.693844854s 6.13264221s
2.574230731s 6.857576762s

What did you expect to see?

Expected to see RWLock() to be atleast twice as fast than Lock()

What did you see instead?

Lock() is faster than RWLock() by almost 2X, then what's the point of having the RWMutex in the first place ?
@gaurav1086 gaurav1086 mentioned this issue May 2, 2020
Closed
@ianlancetaylor ianlancetaylor changed the title sync.RWMutex::RLock() is slower than sync.Mutex::Lock() by 2X sync: RWMutex.RLock is 2x slower than Mutex.Lock May 2, 2020
@ianlancetaylor
Copy link
Contributor

RWMutex of course lets multiple goroutines hold the read lock in parallel, so there is a reason to have it even if it is slower. It's pretty much sure to be slower than Mutex since it has a more complicated job to do. But 2x slower does seem a bit surprising.

@ianlancetaylor
Copy link
Contributor

I suspect that your test is more about collisions on the single output file when using RWMutex than it is about a comparison between RWMutex and Mutex.

@gaurav1086
Copy link
Contributor Author

@ianlancetaylor , thanks for the analysis and valuable insights. That makes sense to me. So the single output file really became the point of contention for the goroutines. I did another experiment and that validates your point. Now, instead of writing to a file, I simply append the values to a temporary slice within the goroutine (which will be automatically garbage collected) and now the results make more sense.

Benchmark results:
Locks RWLocks
19.174382ms 6.887933ms
18.217681ms 6.904993ms
16.707576ms 12.16411ms
16.437133ms 12.819353ms
15.81977ms 7.775752ms
17.277763ms 12.772043ms
16.645841ms 14.092959ms
17.154119ms 6.893917ms

code link: https://play.golang.com/p/zf0n9ytgH4E

@davecheney
Copy link
Contributor

Id be concerned that your program is benchmarking heap growth and gc performance.

Have you tried rewriting your benchmark has a regular testing.B style benchmark. If you did that you would benefit from the tools like -count, -cpu and benchstat.

@mvdan mvdan added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance labels May 3, 2020
@ianlancetaylor
Copy link
Contributor

Thanks. I don't think there is any action to take here, so I'm inclined to close this issue. We're always interested in performance improvements, of course, but I don't see anything here that leads to any promising approach to better performance.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label May 4, 2020
@gaurav1086
Copy link
Contributor Author

@ianlancetaylor , sure I agree. Thanks again for the clarification.

@bcmills
Copy link
Contributor

bcmills commented May 4, 2020

RWMutex could stand a bit of an overhaul in general. (See also #17973.)

@fho fho mentioned this issue Dec 15, 2020
Open
@golang golang locked and limited conversation to collaborators May 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@davecheney @gaurav1086 @ianlancetaylor @mvdan @bcmills @gopherbot