Skip to content

Commit

Permalink
runtime: avoid repeated findmoduledatap calls
Browse files Browse the repository at this point in the history
Currently almost every function that deals with a *_func has to first
look up the *moduledata for the module containing the function's entry
point. This means we almost always do at least two identical module
lookups whenever we deal with a *_func (one to get the *_func and
another to get something from its module data) and sometimes several
more.

Fix this by making findfunc return a new funcInfo type that embeds
*_func, but also includes the *moduledata, and making all of the
functions that currently take a *_func instead take a funcInfo and use
the already-found *moduledata.

This transformation is trivial for the most part, since the *_func
type is usually inferred. The annoying part is that we can no longer
use nil to indicate failure, so this introduces a funcInfo.valid()
method and replaces nil checks with calls to valid.

Change-Id: I9b8075ef1c31185c1943596d96dec45c7ab5100f
Reviewed-on: https://go-review.googlesource.com/37331
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
  • Loading branch information
aclements committed Mar 6, 2017
1 parent 6533cc1 commit 0efc8b2
Show file tree
Hide file tree
Showing 20 changed files with 84 additions and 73 deletions.
4 changes: 2 additions & 2 deletions src/runtime/extern.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func Caller(skip int) (pc uintptr, file string, line int, ok bool) {
return
}
f := findfunc(rpc[1])
if f == nil {
if !f.valid() {
// TODO(rsc): Probably a bug?
// The C version said "have retpc at least"
// but actually returned pc=0.
Expand All @@ -187,7 +187,7 @@ func Caller(skip int) (pc uintptr, file string, line int, ok bool) {
// All architectures turn faults into apparent calls to sigpanic.
// If we see a call to sigpanic, we do not back up the PC to find
// the line number of the call instruction, because there is no call.
if xpc > f.entry && (g == nil || g.entry != funcPC(sigpanic)) {
if xpc > f.entry && (!g.valid() || g.entry != funcPC(sigpanic)) {
xpc--
}
file, line32 := funcline(f, xpc)
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/heapdump.go
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ func dumpmemprof_callback(b *bucket, nstk uintptr, pstk *uintptr, size, allocs,
for i := uintptr(0); i < nstk; i++ {
pc := stk[i]
f := findfunc(pc)
if f == nil {
if !f.valid() {
var buf [64]byte
n := len(buf)
n--
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/mbitmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -1877,7 +1877,7 @@ func getgcmask(ep interface{}) (mask []byte) {
frame.sp = uintptr(p)
_g_ := getg()
gentraceback(_g_.m.curg.sched.pc, _g_.m.curg.sched.sp, 0, _g_.m.curg, 0, nil, 1000, getgcmaskcb, noescape(unsafe.Pointer(&frame)), 0)
if frame.fn != nil {
if frame.fn.valid() {
f := frame.fn
targetpc := frame.continpc
if targetpc == 0 {
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/os3_plan9.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func sighandler(_ureg *ureg, note *byte, gp *g) int {
// but we do recognize the top pointer on the stack as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(*(*uintptr)(unsafe.Pointer(sp))) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(*(*uintptr)(unsafe.Pointer(sp))).valid() {
pc = 0
}

Expand Down
4 changes: 2 additions & 2 deletions src/runtime/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func pluginftabverify(md *moduledata) {
continue
}

f := (*_func)(unsafe.Pointer(&md.pclntable[md.ftab[i].funcoff]))
f := funcInfo{(*_func)(unsafe.Pointer(&md.pclntable[md.ftab[i].funcoff])), md}
name := funcname(f)

// A common bug is f.entry has a relocation to a duplicate
Expand All @@ -104,7 +104,7 @@ func pluginftabverify(md *moduledata) {
name2 := "none"
entry2 := uintptr(0)
f2 := findfunc(entry)
if f2 != nil {
if f2.valid() {
name2 = funcname(f2)
entry2 = f2.entry
}
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3336,7 +3336,7 @@ func sigprofNonGoPC(pc uintptr) {
// or putting one on the stack at the right offset.
func setsSP(pc uintptr) bool {
f := findfunc(pc)
if f == nil {
if !f.valid() {
// couldn't find the function for this PC,
// so assume the worst and stop traceback
return true
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/race.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func raceSymbolizeCode(ctx *symbolizeCodeContext) {
if f != nil {
file, line := f.FileLine(ctx.pc)
if line != 0 {
ctx.fn = cfuncname(f.raw())
ctx.fn = cfuncname(f.funcInfo())
ctx.line = uintptr(line)
ctx.file = &bytes(file)[0] // assume NUL-terminated
ctx.off = ctx.pc - f.Entry()
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/runtime2.go
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ type _panic struct {

// stack traces
type stkframe struct {
fn *_func // function being run
fn funcInfo // function being run
pc uintptr // program counter within fn
continpc uintptr // program counter where execution can continue, or 0 if not
lr uintptr // program counter at caller aka link register
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_386.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the top pointer on the stack as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(*(*uintptr)(unsafe.Pointer(sp))) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(*(*uintptr)(unsafe.Pointer(sp))).valid() {
pc = 0
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_amd64x.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the top pointer on the stack as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(*(*uintptr)(unsafe.Pointer(sp))) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(*(*uintptr)(unsafe.Pointer(sp))).valid() {
pc = 0
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_arm.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the link register as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(uintptr(c.lr())) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.lr())).valid() {
pc = 0
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_arm64.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the link register as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(uintptr(c.lr())) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.lr())).valid() {
pc = 0
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_linux_s390x.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the link register as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(uintptr(c.link())) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
pc = 0
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_mips64x.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the link register as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(uintptr(c.link())) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
pc = 0
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_mipsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the link register as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(uintptr(c.link())) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
pc = 0
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/signal_ppc64x.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
// but we do recognize the link register as code,
// then assume this was a call to non-code and treat like
// pc == 0, to make unwinding show the context.
if pc != 0 && findfunc(pc) == nil && findfunc(uintptr(c.link())) != nil {
if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
pc = 0
}

Expand Down
6 changes: 3 additions & 3 deletions src/runtime/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ func ptrbit(bv *gobitvector, i uintptr) uint8 {

// bv describes the memory starting at address scanp.
// Adjust any pointers contained therein.
func adjustpointers(scanp unsafe.Pointer, cbv *bitvector, adjinfo *adjustinfo, f *_func) {
func adjustpointers(scanp unsafe.Pointer, cbv *bitvector, adjinfo *adjustinfo, f funcInfo) {
bv := gobv(*cbv)
minp := adjinfo.old.lo
maxp := adjinfo.old.hi
Expand All @@ -589,7 +589,7 @@ func adjustpointers(scanp unsafe.Pointer, cbv *bitvector, adjinfo *adjustinfo, f
pp := (*uintptr)(add(scanp, i*sys.PtrSize))
retry:
p := *pp
if f != nil && 0 < p && p < minLegalPointer && debug.invalidptr != 0 {
if f.valid() && 0 < p && p < minLegalPointer && debug.invalidptr != 0 {
// Looks like a junk value in a pointer slot.
// Live analysis wrong?
getg().m.traceback = 2
Expand Down Expand Up @@ -713,7 +713,7 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool {
if stackDebug >= 3 {
print(" args\n")
}
adjustpointers(unsafe.Pointer(frame.argp), &bv, adjinfo, nil)
adjustpointers(unsafe.Pointer(frame.argp), &bv, adjinfo, funcInfo{})
}
return true
}
Expand Down
Loading

0 comments on commit 0efc8b2

Please sign in to comment.