Skip to content

fix extra heap allocations when detector is disabled#2

Merged
algorandskiy merged 2 commits intomasterfrom
pavel/lock-allocation-fix
Sep 27, 2023
Merged

fix extra heap allocations when detector is disabled#2
algorandskiy merged 2 commits intomasterfrom
pavel/lock-allocation-fix

Conversation

@algorandskiy
Copy link
Copy Markdown

Summary

I found deadlock detector has one allocation overhead per locker call b/c it passes Lock() method to the lock helper, and it escapes.

Benchmark

Before:

go test ./ -run ^$ -bench BenchmarkRWMutexAlloc -benchmem -memprofile=mem.pprof
goos: darwin
goarch: arm64
pkg: github.com/algorand/go-deadlock
BenchmarkRWMutexAlloc-10    	32553236	        36.69 ns/op	      16 B/op	       1 allocs/op
PASS
ok  	github.com/algorand/go-deadlock	2.273s

After:

go test ./ -run ^$ -bench BenchmarkRWMutexAlloc -benchmem -memprofile=mem.pprof
goos: darwin
goarch: arm64
pkg: github.com/algorand/go-deadlock
BenchmarkRWMutexAlloc-10    	45803848	        26.02 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/algorand/go-deadlock	2.201s

Comment thread deadlock.go Outdated
lock(m.mu.Lock, m, false)

// shortcut for disabled deadlock detection to prevent extra copying of `m.mu.Lock` to the heap
if Opts.Disable {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if this check was moved above the counterMu lock?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@algorandskiy algorandskiy requested a review from cce September 27, 2023 14:23
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

Successfully merging this pull request may close these issues.

2 participants