Skip to content

Commit e2ca417

Browse files
committed
cmd/link: linker portion of dead map removal
This patch contains the linker changes needed to enable deadcoding of large unreferenced map variables, in combination with a previous compiler change. We add a new cleanup function that runs just after deadcode that looks for relocations in "init" funcs that are weak, of type R_CALL (and siblings), and are targeting an unreachable function. If we find such a relocation, after checking to make sure it targets a map.init.XXX helper, we redirect the relocation to a point to a no-op routine ("mapinitnoop") in the runtime. Compilebench results for this change: │ out.base.txt │ out.wrap.txt │ │ sec/op │ sec/op vs base │ Template 218.6m ± 2% 221.1m ± 1% ~ (p=0.129 n=39) Unicode 180.5m ± 1% 178.9m ± 1% -0.93% (p=0.006 n=39) GoTypes 1.162 ± 1% 1.156 ± 1% ~ (p=0.850 n=39) Compiler 143.6m ± 1% 142.6m ± 1% ~ (p=0.743 n=39) SSA 8.698 ± 1% 8.719 ± 1% ~ (p=0.145 n=39) Flate 142.6m ± 1% 143.9m ± 3% ~ (p=0.287 n=39) GoParser 247.7m ± 1% 248.8m ± 1% ~ (p=0.265 n=39) Reflect 588.0m ± 1% 590.4m ± 1% ~ (p=0.269 n=39) Tar 198.5m ± 1% 201.3m ± 1% +1.38% (p=0.005 n=39) XML 259.1m ± 1% 260.0m ± 1% ~ (p=0.376 n=39) LinkCompiler 746.8m ± 2% 747.8m ± 1% ~ (p=0.706 n=39) ExternalLinkCompiler 1.906 ± 0% 1.902 ± 1% ~ (p=0.207 n=40) LinkWithoutDebugCompiler 522.4m ± 21% 471.1m ± 1% -9.81% (p=0.000 n=40) StdCmd 21.32 ± 0% 21.39 ± 0% +0.32% (p=0.035 n=40) geomean 609.2m 606.0m -0.53% │ out.base.txt │ out.wrap.txt │ │ user-sec/op │ user-sec/op vs base │ Template 401.9m ± 3% 406.9m ± 2% ~ (p=0.291 n=39) Unicode 191.9m ± 6% 196.1m ± 3% ~ (p=0.052 n=39) GoTypes 3.967 ± 3% 4.056 ± 1% ~ (p=0.099 n=39) Compiler 171.1m ± 3% 173.4m ± 3% ~ (p=0.415 n=39) SSA 30.04 ± 4% 30.25 ± 4% ~ (p=0.106 n=39) Flate 246.3m ± 3% 248.9m ± 4% ~ (p=0.499 n=39) GoParser 518.7m ± 1% 520.6m ± 2% ~ (p=0.531 n=39) Reflect 1.670 ± 1% 1.656 ± 2% ~ (p=0.137 n=39) Tar 352.7m ± 2% 360.3m ± 2% ~ (p=0.117 n=39) XML 528.8m ± 2% 521.1m ± 2% ~ (p=0.296 n=39) LinkCompiler 1.128 ± 2% 1.140 ± 2% ~ (p=0.324 n=39) ExternalLinkCompiler 2.165 ± 2% 2.147 ± 2% ~ (p=0.537 n=40) LinkWithoutDebugCompiler 484.2m ± 4% 490.7m ± 3% ~ (p=0.897 n=40) geomean 818.5m 825.1m +0.80% │ out.base.txt │ out.wrap.txt │ │ text-bytes │ text-bytes vs base │ HelloSize 766.0Ki ± 0% 766.0Ki ± 0% ~ (p=1.000 n=40) ¹ CmdGoSize 10.02Mi ± 0% 10.02Mi ± 0% -0.03% (n=40) geomean 2.738Mi 2.738Mi -0.01% ¹ all samples are equal │ out.base.txt │ out.wrap.txt │ │ data-bytes │ data-bytes vs base │ HelloSize 14.17Ki ± 0% 14.17Ki ± 0% ~ (p=1.000 n=40) ¹ CmdGoSize 308.3Ki ± 0% 298.5Ki ± 0% -3.19% (n=40) geomean 66.10Ki 65.04Ki -1.61% ¹ all samples are equal │ out.base.txt │ out.wrap.txt │ │ bss-bytes │ bss-bytes vs base │ HelloSize 197.3Ki ± 0% 197.3Ki ± 0% ~ (p=1.000 n=40) ¹ CmdGoSize 228.2Ki ± 0% 228.1Ki ± 0% -0.01% (n=40) geomean 212.2Ki 212.1Ki -0.01% ¹ all samples are equal │ out.base.txt │ out.wrap.txt │ │ exe-bytes │ exe-bytes vs base │ HelloSize 1.192Mi ± 0% 1.192Mi ± 0% +0.00% (p=0.000 n=40) CmdGoSize 14.85Mi ± 0% 14.83Mi ± 0% -0.09% (n=40) geomean 4.207Mi 4.205Mi -0.05% Also tested for any linker changes by benchmarking relink of k8s "kubelet"; no changes to speak of there. Updates #2559. Updates #36021. Updates #14840. Change-Id: I4cc5370b3f20679a1065aaaf87bdf2881e257631 Reviewed-on: https://go-review.googlesource.com/c/go/+/463395 Run-TryBot: Than McIntosh <[email protected]> Reviewed-by: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent b46e44a commit e2ca417

File tree

7 files changed

+103
-15
lines changed

7 files changed

+103
-15
lines changed

src/cmd/compile/internal/ssagen/pgen.go

-4
Original file line numberDiff line numberDiff line change
@@ -240,10 +240,6 @@ func RegisterMapInitLsym(s *obj.LSym) {
240240
// outlined global map initializer functions; if it finds any such
241241
// relocs, it flags them as R_WEAK.
242242
func weakenGlobalMapInitRelocs(fn *ir.Func) {
243-
// Disabled until next patch.
244-
if true {
245-
return
246-
}
247243
if globalMapInitLsyms == nil {
248244
return
249245
}

src/cmd/link/internal/ld/deadcode.go

+48
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"cmd/link/internal/sym"
1313
"fmt"
1414
"internal/buildcfg"
15+
"strings"
1516
"unicode"
1617
)
1718

@@ -29,6 +30,8 @@ type deadcodePass struct {
2930
dynlink bool
3031

3132
methodsigstmp []methodsig // scratch buffer for decoding method signatures
33+
pkginits []loader.Sym
34+
mapinitnoop loader.Sym
3235
}
3336

3437
func (d *deadcodePass) init() {
@@ -105,6 +108,11 @@ func (d *deadcodePass) init() {
105108
}
106109
d.mark(s, 0)
107110
}
111+
112+
d.mapinitnoop = d.ldr.Lookup("runtime.mapinitnoop", abiInternalVer)
113+
if d.mapinitnoop == 0 {
114+
panic("could not look up runtime.mapinitnoop")
115+
}
108116
}
109117

110118
func (d *deadcodePass) flood() {
@@ -229,6 +237,12 @@ func (d *deadcodePass) flood() {
229237
}
230238
d.mark(a.Sym(), symIdx)
231239
}
240+
// Record sym if package init func (here naux != 0 is a cheap way
241+
// to check first if it is a function symbol).
242+
if naux != 0 && d.ldr.IsPkgInit(symIdx) {
243+
244+
d.pkginits = append(d.pkginits, symIdx)
245+
}
232246
// Some host object symbols have an outer object, which acts like a
233247
// "carrier" symbol, or it holds all the symbols for a particular
234248
// section. We need to mark all "referenced" symbols from that carrier,
@@ -262,6 +276,37 @@ func (d *deadcodePass) flood() {
262276
}
263277
}
264278

279+
// mapinitcleanup walks all pkg init functions and looks for weak relocations
280+
// to mapinit symbols that are no longer reachable. It rewrites
281+
// the relocs to target a new no-op routine in the runtime.
282+
func (d *deadcodePass) mapinitcleanup() {
283+
for _, idx := range d.pkginits {
284+
relocs := d.ldr.Relocs(idx)
285+
var su *loader.SymbolBuilder
286+
for i := 0; i < relocs.Count(); i++ {
287+
r := relocs.At(i)
288+
rs := r.Sym()
289+
if r.Weak() && r.Type().IsDirectCall() && !d.ldr.AttrReachable(rs) {
290+
// double check to make sure target is indeed map.init
291+
rsn := d.ldr.SymName(rs)
292+
if !strings.Contains(rsn, "map.init") {
293+
panic(fmt.Sprintf("internal error: expected map.init sym for weak call reloc, got %s -> %s", d.ldr.SymName(idx), rsn))
294+
}
295+
d.ldr.SetAttrReachable(d.mapinitnoop, true)
296+
if d.ctxt.Debugvlog > 1 {
297+
d.ctxt.Logf("deadcode: %s rewrite %s ref to %s\n",
298+
d.ldr.SymName(idx), rsn,
299+
d.ldr.SymName(d.mapinitnoop))
300+
}
301+
if su == nil {
302+
su = d.ldr.MakeSymbolUpdater(idx)
303+
}
304+
su.SetRelocSym(i, d.mapinitnoop)
305+
}
306+
}
307+
}
308+
}
309+
265310
func (d *deadcodePass) mark(symIdx, parent loader.Sym) {
266311
if symIdx != 0 && !d.ldr.AttrReachable(symIdx) {
267312
d.wq.push(symIdx)
@@ -370,6 +415,9 @@ func deadcode(ctxt *Link) {
370415
}
371416
d.flood()
372417
}
418+
if *flagPruneWeakMap {
419+
d.mapinitcleanup()
420+
}
373421
}
374422

375423
// methodsig is a typed method signature (name + type).

src/cmd/link/internal/ld/deadcode_test.go

+17-11
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,16 @@ func TestDeadcode(t *testing.T) {
1919

2020
tests := []struct {
2121
src string
22-
pos, neg string // positive and negative patterns
22+
pos, neg []string // positive and negative patterns
2323
}{
24-
{"reflectcall", "", "main.T.M"},
25-
{"typedesc", "", "type:main.T"},
26-
{"ifacemethod", "", "main.T.M"},
27-
{"ifacemethod2", "main.T.M", ""},
28-
{"ifacemethod3", "main.S.M", ""},
29-
{"ifacemethod4", "", "main.T.M"},
24+
{"reflectcall", nil, []string{"main.T.M"}},
25+
{"typedesc", nil, []string{"type:main.T"}},
26+
{"ifacemethod", nil, []string{"main.T.M"}},
27+
{"ifacemethod2", []string{"main.T.M"}, nil},
28+
{"ifacemethod3", []string{"main.S.M"}, nil},
29+
{"ifacemethod4", nil, []string{"main.T.M"}},
30+
{"globalmap", []string{"main.small", "main.effect"},
31+
[]string{"main.large"}},
3032
}
3133
for _, test := range tests {
3234
test := test
@@ -39,11 +41,15 @@ func TestDeadcode(t *testing.T) {
3941
if err != nil {
4042
t.Fatalf("%v: %v:\n%s", cmd.Args, err, out)
4143
}
42-
if test.pos != "" && !bytes.Contains(out, []byte(test.pos+"\n")) {
43-
t.Errorf("%s should be reachable. Output:\n%s", test.pos, out)
44+
for _, pos := range test.pos {
45+
if !bytes.Contains(out, []byte(pos+"\n")) {
46+
t.Errorf("%s should be reachable. Output:\n%s", pos, out)
47+
}
4448
}
45-
if test.neg != "" && bytes.Contains(out, []byte(test.neg+"\n")) {
46-
t.Errorf("%s should not be reachable. Output:\n%s", test.neg, out)
49+
for _, neg := range test.neg {
50+
if bytes.Contains(out, []byte(neg+"\n")) {
51+
t.Errorf("%s should not be reachable. Output:\n%s", neg, out)
52+
}
4753
}
4854
})
4955
}

src/cmd/link/internal/ld/main.go

+1
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ var (
100100
FlagRound = flag.Int("R", -1, "set address rounding `quantum`")
101101
FlagTextAddr = flag.Int64("T", -1, "set text segment `address`")
102102
flagEntrySymbol = flag.String("E", "", "set `entry` symbol name")
103+
flagPruneWeakMap = flag.Bool("pruneweakmap", true, "prune weak mapinit refs")
103104
cpuprofile = flag.String("cpuprofile", "", "write cpu profile to `file`")
104105
memprofile = flag.String("memprofile", "", "write memory profile to `file`")
105106
memprofilerate = flag.Int64("memprofilerate", 0, "set runtime.MemProfileRate to `rate`")
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package main
2+
3+
import "os"
4+
5+
// Too small to trigger deadcode (currently)
6+
var small = map[string]int{"foo": 1}
7+
8+
// Has side effects, which prevent deadcode
9+
var effect = map[string]int{"foo": os.Getpid()}
10+
11+
// Large and side-effect free
12+
var large = map[string]int{
13+
"11": 1, "12": 2, "13": 3, "14": 4, "15": 5, "16": 6, "17": 7, "18": 8, "19": 9, "110": 10,
14+
"21": 1, "22": 2, "23": 3, "24": 4, "25": 5, "26": 6, "27": 7, "28": 8, "29": 9, "210": 10,
15+
"31": 1, "32": 2, "33": 3, "34": 4, "35": 5, "36": 6, "37": 7, "38": 8, "39": 9, "310": 10,
16+
"41": 1, "42": 2, "43": 3, "44": 4, "45": 5, "46": 6, "47": 7, "48": 8, "49": 9, "410": 10,
17+
"51": 1, "52": 2, "53": 3, "54": 4, "55": 5, "56": 6, "57": 7, "58": 8, "59": 9, "510": 10,
18+
"61": 1, "62": 2, "63": 3, "64": 4, "65": 5, "66": 6, "67": 7, "68": 8, "69": 9, "610": 10,
19+
"71": 1, "72": 2, "73": 3, "74": 4, "75": 5, "76": 6, "77": 7, "78": 8, "79": 9, "710": 10,
20+
"81": 1, "82": 2, "83": 3, "84": 4, "85": 5, "86": 6, "87": 7, "88": 8, "89": 9, "810": 10,
21+
"91": 1, "92": 2, "93": 3, "94": 4, "95": 5, "96": 6, "97": 7, "98": 8, "99": 9, "910": 10,
22+
"101": 1, "102": 2, "103": 3, "104": 4, "105": 5, "106": 6, "107": 7, "108": 8, "109": 9, "1010": 10, "1021": 2,
23+
}
24+
25+
func main() {
26+
}

src/runtime/asm.s

+4
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,7 @@
88
TEXT ·sigpanic0(SB),NOSPLIT,$0-0
99
JMP ·sigpanic<ABIInternal>(SB)
1010
#endif
11+
12+
// See map.go comment on the need for this routine.
13+
TEXT ·mapinitnoop<ABIInternal>(SB),NOSPLIT,$0-0
14+
RET

src/runtime/map.go

+7
Original file line numberDiff line numberDiff line change
@@ -1422,3 +1422,10 @@ func reflectlite_maplen(h *hmap) int {
14221422

14231423
const maxZero = 1024 // must match value in reflect/value.go:maxZero cmd/compile/internal/gc/walk.go:zeroValSize
14241424
var zeroVal [maxZero]byte
1425+
1426+
// mapinitnoop is a no-op function known the Go linker; if a given global
1427+
// map (of the right size) is determined to be dead, the linker will
1428+
// rewrite the relocation (from the package init func) from the outlined
1429+
// map init function to this symbol. Defined in assembly so as to avoid
1430+
// complications with instrumentation (coverage, etc).
1431+
func mapinitnoop()

0 commit comments

Comments
 (0)