Skip to content

Commit

Permalink
Reduce overhead by 90% (#2)
Browse files Browse the repository at this point in the history
* Simplify disabled fast-path

* Avoid `defer`s and don't hold lock across `getGID()` calls

This reduces runtime overhead by about 45% in CockroachDB.

* Use github.com/petermattis/goid for efficient goroutine ID lookup

 This reduces runtime overhead by about 80% in CockroachDB.

* Add Travis CI
  • Loading branch information
tamird authored and sasha-s committed Dec 1, 2016
1 parent 09aefc0 commit 3bab8aa
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 38 deletions.
8 changes: 8 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
language: go

go:
- 1.3
- 1.4
- 1.5
- 1.6
- 1.7
59 changes: 21 additions & 38 deletions deadlock.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (
"fmt"
"io"
"os"
"runtime"
"strconv"
"sync"
"time"

"github.com/petermattis/goid"
)

// Opts control how deadlock detection behaves.
Expand Down Expand Up @@ -62,12 +62,10 @@ func (m *Mutex) Lock() {
// It is allowed for one goroutine to lock a Mutex and then
// arrange for another goroutine to unlock it.
func (m *Mutex) Unlock() {
if Opts.Disable {
m.mu.Unlock()
return
}
m.mu.Unlock()
PostUnlock(m)
if !Opts.Disable {
PostUnlock(m)
}
}

// An RWMutex is a drop-in replacement for sync.RWMutex.
Expand Down Expand Up @@ -96,12 +94,10 @@ func (m *RWMutex) Lock() {
// goroutine. One goroutine may RLock (Lock) an RWMutex and then
// arrange for another goroutine to RUnlock (Unlock) it.
func (m *RWMutex) Unlock() {
if Opts.Disable {
m.mu.Unlock()
return
}
m.mu.Unlock()
PostUnlock(m)
if !Opts.Disable {
PostUnlock(m)
}
}

// RLock locks the mutex for reading.
Expand All @@ -117,12 +113,10 @@ func (m *RWMutex) RLock() {
// It is a run-time error if rw is not locked for reading
// on entry to RUnlock.
func (m *RWMutex) RUnlock() {
if Opts.Disable {
m.mu.RUnlock()
return
if !Opts.Disable {
PostUnlock(m)
}
m.mu.RUnlock()
PostUnlock(m)
}

// RLocker returns a Locker interface that implements
Expand Down Expand Up @@ -173,13 +167,13 @@ func lock(lockFn func(), ptr interface{}) {
fmt.Fprintf(Opts.LogBuf, "goroutine %v lock %p\n", prev.gid, ptr)
printStack(Opts.LogBuf, prev.stack)
fmt.Fprintln(Opts.LogBuf, "Have been trying to lock it again for more than", Opts.DeadlockTimeout)
fmt.Fprintf(Opts.LogBuf, "goroutine %v lock %p\n", getGID(), ptr)
fmt.Fprintf(Opts.LogBuf, "goroutine %v lock %p\n", goid.Get(), ptr)
printStack(Opts.LogBuf, callers(2))
fmt.Fprintln(Opts.LogBuf)
stacks := stacks()
grs := bytes.Split(stacks, []byte("\n\n"))
for _, g := range grs {
if extractGID(g) == prev.gid {
if goid.ExtractGID(g) == prev.gid {
fmt.Fprintln(Opts.LogBuf, "Here is what goroutine", prev.gid, "doing now")
Opts.LogBuf.Write(g)
fmt.Fprintln(Opts.LogBuf)
Expand Down Expand Up @@ -210,7 +204,7 @@ type lockOrder struct {

type stackGID struct {
stack []uintptr
gid uint64
gid int64
}

type beforeAfter struct {
Expand All @@ -233,19 +227,20 @@ func newLockOrder() *lockOrder {
}

func (l *lockOrder) PostLock(skip int, p interface{}) {
stack := callers(skip)
gid := goid.Get()
l.mu.Lock()
defer l.mu.Unlock()
l.cur[p] = stackGID{callers(skip), getGID()}
l.cur[p] = stackGID{stack, gid}
l.mu.Unlock()
}

func (l *lockOrder) PreLock(skip int, p interface{}) {
if Opts.DisableLockOrderDetection {
return
}
l.mu.Lock()
defer l.mu.Unlock()
stack := callers(skip)
gid := getGID()
gid := goid.Get()
l.mu.Lock()
for b, bs := range l.cur {
if b == p {
continue
Expand All @@ -272,12 +267,13 @@ func (l *lockOrder) PreLock(skip int, p interface{}) {
}
}
l.cur[p] = stackGID{stack, gid}
l.mu.Unlock()
}

func (l *lockOrder) PostUnlock(p interface{}) {
l.mu.Lock()
defer l.mu.Unlock()
delete(l.cur, p)
l.mu.Unlock()
}

type rlocker RWMutex
Expand All @@ -298,17 +294,4 @@ func (l *lockOrder) other(ptr interface{}) {
fmt.Fprintln(Opts.LogBuf)
}

// Hacky way of getting a goroutine ID.
func getGID() uint64 {
b := make([]byte, 64)
return extractGID(b[:runtime.Stack(b, false)])
}

func extractGID(stack []byte) uint64 {
b := bytes.TrimPrefix(stack, []byte("goroutine "))
b = b[:bytes.IndexByte(b, ' ')]
gid, _ := strconv.ParseUint(string(b), 10, 64)
return gid
}

const header = "POTENTIAL DEADLOCK:"

0 comments on commit 3bab8aa

Please sign in to comment.