Commit 3692925c authored by Josh Bleecher Snyder's avatar Josh Bleecher Snyder

cmd/compile: move Text.From.Sym initialization earlier

The initialization of an ATEXT Prog's From.Sym
can race with the assemblers in a concurrent compiler.
CL 40254 contains an initial, failed attempt to
fix that race.

This CL takes a different approach: Rather than
expose an API to initialize the Prog,
expose an API to initialize the Sym.

The initialization of the Sym can then be
moved earlier in the compiler, avoiding the race.

The growth of gc.Func has negligible
performance impact; see below.

Passes toolstash -cmp.

Updates #15756

name       old alloc/op      new alloc/op      delta
Template        38.8MB ± 0%       38.8MB ± 0%    ~     (p=0.968 n=9+10)
Unicode         29.8MB ± 0%       29.8MB ± 0%    ~     (p=0.684 n=10+10)
GoTypes          113MB ± 0%        113MB ± 0%    ~     (p=0.912 n=10+10)
SSA             1.25GB ± 0%       1.25GB ± 0%    ~     (p=0.481 n=10+10)
Flate           25.3MB ± 0%       25.3MB ± 0%    ~     (p=0.105 n=10+10)
GoParser        31.7MB ± 0%       31.8MB ± 0%  +0.09%  (p=0.016 n=8+10)
Reflect         78.3MB ± 0%       78.2MB ± 0%    ~     (p=0.190 n=10+10)
Tar             26.5MB ± 0%       26.6MB ± 0%  +0.13%  (p=0.011 n=10+10)
XML             42.4MB ± 0%       42.4MB ± 0%    ~     (p=0.971 n=10+10)

name       old allocs/op     new allocs/op     delta
Template          378k ± 1%         378k ± 0%    ~     (p=0.315 n=10+9)
Unicode           321k ± 1%         321k ± 0%    ~     (p=0.436 n=10+10)
GoTypes          1.14M ± 0%        1.14M ± 0%    ~     (p=0.079 n=10+9)
SSA              9.70M ± 0%        9.70M ± 0%  -0.04%  (p=0.035 n=10+10)
Flate             233k ± 1%         234k ± 1%    ~     (p=0.529 n=10+10)
GoParser          315k ± 0%         316k ± 0%    ~     (p=0.095 n=9+10)
Reflect           980k ± 0%         980k ± 0%    ~     (p=0.436 n=10+10)
Tar               249k ± 1%         250k ± 0%    ~     (p=0.280 n=10+10)
XML               391k ± 1%         391k ± 1%    ~     (p=0.481 n=10+10)

Change-Id: I3c93033dddd2e1df8cc54a106a6e615d27859e71
Reviewed-on: https://go-review.googlesource.com/40496
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarCherry Zhang <cherryyz@google.com>
parent 7faf3024
......@@ -160,6 +160,7 @@ func (p *Parser) asmText(word string, operands [][]lex.Token) {
}
argSize = p.positiveAtoi(op[1].String())
}
p.ctxt.InitTextSym(nameAddr.Sym, int(flag))
prog := &obj.Prog{
Ctxt: p.ctxt,
As: obj.ATEXT,
......@@ -171,9 +172,8 @@ func (p *Parser) asmText(word string, operands [][]lex.Token) {
// Argsize set below.
},
}
nameAddr.Sym.Text = prog
prog.To.Val = int32(argSize)
p.ctxt.InitTextSym(prog, int(flag))
p.append(prog, "", true)
}
......
......@@ -144,34 +144,45 @@ func (pp *Progs) settext(fn *Node) {
if pp.Text != nil {
Fatalf("Progs.settext called twice")
}
ptxt := pp.Prog(obj.ATEXT)
if nam := fn.Func.Nname; !isblank(nam) {
if fn.Func.lsym != nil {
fn.Func.lsym.Text = ptxt
ptxt.From.Type = obj.TYPE_MEM
ptxt.From.Name = obj.NAME_EXTERN
ptxt.From.Sym = Linksym(nam.Sym)
if fn.Func.Pragma&Systemstack != 0 {
ptxt.From.Sym.Set(obj.AttrCFunc, true)
ptxt.From.Sym = fn.Func.lsym
}
pp.Text = ptxt
}
func (f *Func) initLSym() {
if f.lsym != nil {
Fatalf("Func.initLSym called twice")
}
if nam := f.Nname; !isblank(nam) {
f.lsym = Linksym(nam.Sym)
if f.Pragma&Systemstack != 0 {
f.lsym.Set(obj.AttrCFunc, true)
}
}
var flag int
if fn.Func.Dupok() {
if f.Dupok() {
flag |= obj.DUPOK
}
if fn.Func.Wrapper() {
if f.Wrapper() {
flag |= obj.WRAPPER
}
if fn.Func.NoFramePointer() {
if f.NoFramePointer() {
flag |= obj.NOFRAME
}
if fn.Func.Needctxt() {
if f.Needctxt() {
flag |= obj.NEEDCTXT
}
if fn.Func.Pragma&Nosplit != 0 {
if f.Pragma&Nosplit != 0 {
flag |= obj.NOSPLIT
}
if fn.Func.ReflectMethod() {
if f.ReflectMethod() {
flag |= obj.REFLECTMETHOD
}
......@@ -179,15 +190,13 @@ func (pp *Progs) settext(fn *Node) {
// See test/recover.go for test cases and src/reflect/value.go
// for the actual functions being considered.
if myimportpath == "reflect" {
switch fn.Func.Nname.Sym.Name {
switch f.Nname.Sym.Name {
case "callReflect", "callMethod":
flag |= obj.WRAPPER
}
}
Ctxt.InitTextSym(ptxt, flag)
pp.Text = ptxt
Ctxt.InitTextSym(f.lsym, flag)
}
func ggloblnod(nam *Node) {
......
......@@ -297,6 +297,9 @@ func compile(fn *Node) {
// From this point, there should be no uses of Curfn. Enforce that.
Curfn = nil
// Set up the function's LSym early to avoid data races with the assemblers.
fn.Func.initLSym()
// Build an SSA backend function.
ssafn := buildssa(fn)
pp := newProgs(fn)
......
......@@ -22,7 +22,7 @@ func TestSizeof(t *testing.T) {
_32bit uintptr // size on 32bit platforms
_64bit uintptr // size on 64bit platforms
}{
{Func{}, 96, 160},
{Func{}, 100, 168},
{Name{}, 36, 56},
{Param{}, 28, 56},
{Node{}, 84, 136},
......
......@@ -9,6 +9,7 @@ package gc
import (
"cmd/compile/internal/syntax"
"cmd/compile/internal/types"
"cmd/internal/obj"
"cmd/internal/src"
)
......@@ -331,6 +332,7 @@ type Func struct {
Top int // top context (Ecall, Eproc, etc)
Closure *Node // OCLOSURE <-> ODCLFUNC
Nname *Node
lsym *obj.LSym
Inl Nodes // copy of the body for use in inlining
InlCost int32
......
......@@ -112,11 +112,7 @@ func Flushplist(ctxt *Link, plist *Plist, newprog ProgAlloc) {
ctxt.Text = append(ctxt.Text, text...)
}
func (ctxt *Link) InitTextSym(p *Prog, flag int) {
if p.As != ATEXT {
ctxt.Diag("InitTextSym non-ATEXT: %v", p)
}
s := p.From.Sym
func (ctxt *Link) InitTextSym(s *LSym, flag int) {
if s == nil {
// func _() { }
return
......@@ -139,7 +135,6 @@ func (ctxt *Link) InitTextSym(p *Prog, flag int) {
s.Set(AttrNeedCtxt, flag&NEEDCTXT != 0)
s.Set(AttrNoFrame, flag&NOFRAME != 0)
s.Type = STEXT
s.Text = p
}
func (ctxt *Link) Globl(s *LSym, size int64, flag int) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment