From d06b0db5bd4c898bd162e16ab603081ab62a527c Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Mon, 14 Mar 2016 15:08:22 -0400 Subject: [PATCH] cmd/link: when pruning methods also prune funcType Remove method type information for pruned methods from any program that does not reflect on methods. This can be a significant saving: addr2line: -310KB (8.8%) A future update might want to consider a more aggressive variant of this: setting the Type and Func fields of reflect.Method to nil for unexported methods. That would shrink cmd/go by 2% and jujud by 2.6% but could be considered an API change. So this CL sticks to the uncontroversial change. For #6853. Change-Id: I5d186d9f822dc118ee89dc572c4912a3b3c72577 Reviewed-on: https://go-review.googlesource.com/20701 Run-TryBot: David Crawshaw TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/cmd/compile/internal/gc/reflect.go | 16 ++---- src/cmd/link/internal/ld/deadcode.go | 74 ++++++++++++++++++-------- 2 files changed, 57 insertions(+), 33 deletions(-) diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index 04367ac86a..8dc1e6cd0b 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -504,18 +504,10 @@ func dextratypeData(s *Sym, ot int, t *Type) int { ot = dgostringptr(s, ot, a.name) ot = dgopkgpath(s, ot, a.pkg) - ot = dsymptr(s, ot, dtypesym(a.mtype), 0) - ot = dsymptr(s, ot, dtypesym(a.type_), 0) - if a.isym != nil { - ot = dmethodptr(s, ot, a.isym) - } else { - ot = duintptr(s, ot, 0) - } - if a.tsym != nil { - ot = dmethodptr(s, ot, a.tsym) - } else { - ot = duintptr(s, ot, 0) - } + ot = dmethodptr(s, ot, dtypesym(a.mtype)) + ot = dmethodptr(s, ot, dtypesym(a.type_)) + ot = dmethodptr(s, ot, a.isym) + ot = dmethodptr(s, ot, a.tsym) } return ot } diff --git a/src/cmd/link/internal/ld/deadcode.go b/src/cmd/link/internal/ld/deadcode.go index a2286eb872..9367375102 100644 --- a/src/cmd/link/internal/ld/deadcode.go +++ b/src/cmd/link/internal/ld/deadcode.go @@ -62,6 +62,12 @@ func deadcode(ctxt *Link) { methSym := Linkrlookup(ctxt, "reflect.Value.Method", 0) reflectSeen := false + if DynlinkingGo() { + // Exported methods may satisfy interfaces we don't know + // about yet when dynamically linking. + reflectSeen = true + } + for { if !reflectSeen { if d.reflectMethod || (callSym != nil && callSym.Attr.Reachable()) || (methSym != nil && methSym.Attr.Reachable()) { @@ -80,6 +86,17 @@ func deadcode(ctxt *Link) { for _, m := range d.markableMethods { if (reflectSeen && m.isExported()) || d.ifaceMethod[m.m] { d.markMethod(m) + } else if reflectSeen { + // This ensures the Type and Func fields of + // reflect.Method are filled as they were in + // Go 1. + // + // An argument could be made for changing this + // and setting those fields to nil. Doing so + // would reduce the binary size of typical + // programs like cmd/go by ~2%. + d.markMethodType(m) + rem = append(rem, m) } else { rem = append(rem, m) } @@ -95,8 +112,9 @@ func deadcode(ctxt *Link) { // Remove all remaining unreached R_METHOD relocations. for _, m := range d.markableMethods { - d.cleanupReloc(m.r0) - d.cleanupReloc(m.r1) + for _, r := range m.r { + d.cleanupReloc(r) + } } if Buildmode != BuildmodeShared { @@ -153,15 +171,19 @@ var markextra = []string{ } // methodref holds the relocations from a receiver type symbol to its -// method. There are two relocations, one for the method type without -// receiver, one with receiver +// method. There are four relocations, one for each of the fields in +// the reflect.method struct: mtyp, typ, ifn, and tfn. type methodref struct { m methodsig - src *LSym // receiver type symbol - r0 *Reloc - r1 *Reloc + src *LSym // receiver type symbol + r [4]*Reloc // R_METHOD relocations to fields of runtime.method } +func (m methodref) mtyp() *LSym { return m.r[0].Sym } +func (m methodref) typ() *LSym { return m.r[1].Sym } +func (m methodref) ifn() *LSym { return m.r[2].Sym } +func (m methodref) tfn() *LSym { return m.r[3].Sym } + func (m methodref) isExported() bool { for _, r := range m.m { return unicode.IsUpper(r) @@ -203,12 +225,18 @@ func (d *deadcodepass) mark(s, parent *LSym) { d.markQueue = append(d.markQueue, s) } -// markMethod marks a method as reachable and preps its R_METHOD relocations. +// markMethod marks a method as reachable. func (d *deadcodepass) markMethod(m methodref) { - d.mark(m.r0.Sym, m.src) - d.mark(m.r1.Sym, m.src) - m.r0.Type = obj.R_ADDR - m.r1.Type = obj.R_ADDR + for _, r := range m.r { + d.mark(r.Sym, m.src) + r.Type = obj.R_ADDR + } +} + +// markMethodType marks just a method's types as reachable. +func (d *deadcodepass) markMethodType(m methodref) { + d.mark(m.mtyp(), m.src) + d.mark(m.typ(), m.src) } // init marks all initial symbols as reachable. @@ -278,6 +306,7 @@ func (d *deadcodepass) flood() { } } + mpos := 0 // 0-3, the R_METHOD relocs of runtime.uncommontype var methods []methodref for i := 0; i < len(s.R); i++ { r := &s.R[i] @@ -290,14 +319,17 @@ func (d *deadcodepass) flood() { } // Collect rtype pointers to methods for // later processing in deadcode. - if len(methods) > 0 { - mref := &methods[len(methods)-1] - if mref.r1 == nil { - mref.r1 = r - continue - } + if mpos == 0 { + m := methodref{src: s} + m.r[0] = r + methods = append(methods, m) + } else { + methods[len(methods)-1].r[mpos] = r + } + mpos++ + if mpos == len(methodref{}.r) { + mpos = 0 } - methods = append(methods, methodref{src: s, r0: r}) } if len(methods) > 0 { // Decode runtime type information for type methods @@ -310,8 +342,8 @@ func (d *deadcodepass) flood() { for i, m := range methodsigs { name := string(m) name = name[:strings.Index(name, "(")] - if !strings.HasSuffix(methods[i].r0.Sym.Name, name) { - panic(fmt.Sprintf("%q relocation for %q does not match method %q", s.Name, methods[i].r0.Sym.Name, name)) + if !strings.HasSuffix(methods[i].ifn().Name, name) { + panic(fmt.Sprintf("%q relocation for %q does not match method %q", s.Name, methods[i].ifn().Name, name)) } methods[i].m = m } -- 2.48.1