Skip to content

Commit

Permalink
internal/compile: opt: emit a combined tuple for defaults + freevars (g…
Browse files Browse the repository at this point in the history
…oogle#203)

The function knows where to split it.

Also, refactor the common parts of all Functions in the same module---
program, predeclared, globals, constants---into a separate data structure,
module. This reduces the size of Function from 14 words to 8.
  • Loading branch information
adonovan authored May 28, 2019
1 parent 47ec068 commit 3ee1685
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 49 deletions.
2 changes: 1 addition & 1 deletion internal/compile/codegen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestPlusFolding(t *testing.T) {
t.Errorf("#%d: %v", i, err)
continue
}
got := disassemble(Expr(expr, "<expr>", locals))
got := disassemble(Expr(expr, "<expr>", locals).Toplevel)
if test.want != got {
t.Errorf("expression <<%s>> generated <<%s>>, want <<%s>>",
test.src, got, test.want)
Expand Down
35 changes: 20 additions & 15 deletions internal/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import (
const debug = false // TODO(adonovan): use a bitmap of options; and regexp to match files

// Increment this to force recompilation of saved bytecode files.
const Version = 9
const Version = 10

type Opcode uint8

Expand Down Expand Up @@ -121,7 +121,7 @@ const (
CONSTANT // - CONSTANT<constant> value
MAKETUPLE // x1 ... xn MAKETUPLE<n> tuple
MAKELIST // x1 ... xn MAKELIST<n> list
MAKEFUNC // defaults freevars MAKEFUNC<func> fn
MAKEFUNC // defaults+freevars MAKEFUNC<func> fn
LOAD // from1 ... fromN module LOAD<n> v1 ... vN
SETLOCAL // value SETLOCAL<local> -
SETGLOBAL // value SETGLOBAL<global> -
Expand Down Expand Up @@ -253,7 +253,7 @@ var stackEffect = [...]int8{
LT: -1,
LTLT: -1,
MAKEDICT: +1,
MAKEFUNC: -1,
MAKEFUNC: 0,
MAKELIST: variableStackEffect,
MAKETUPLE: variableStackEffect,
MANDATORY: +1,
Expand Down Expand Up @@ -297,7 +297,7 @@ func (op Opcode) String() string {

// A Program is a Starlark file in executable form.
//
// Programs are serialized by the gobProgram function,
// Programs are serialized by the Program.Encode method,
// which must be updated whenever this declaration is changed.
type Program struct {
Loads []Binding // name (really, string) and position of each load stmt
Expand All @@ -310,7 +310,7 @@ type Program struct {

// A Funcode is the code of a compiled Starlark function.
//
// Funcodes are serialized by the gobFunc function,
// Funcodes are serialized by the encoder.function method,
// which must be updated whenever this declaration is changed.
type Funcode struct {
Prog *Program
Expand All @@ -327,6 +327,8 @@ type Funcode struct {
NumKwonlyParams int
HasVarargs, HasKwargs bool

// -- transient state --

lntOnce sync.Once
lnt []pcline // decoded line number table
}
Expand Down Expand Up @@ -454,11 +456,11 @@ func bindings(bindings []*resolve.Binding) []Binding {
return res
}

// Expr compiles an expression to a program consisting of a single toplevel function.
func Expr(expr syntax.Expr, name string, locals []*resolve.Binding) *Funcode {
// Expr compiles an expression to a program whose toplevel function evaluates it.
func Expr(expr syntax.Expr, name string, locals []*resolve.Binding) *Program {
pos := syntax.Start(expr)
stmts := []syntax.Stmt{&syntax.ReturnStmt{Result: expr}}
return File(stmts, pos, name, locals, nil).Toplevel
return File(stmts, pos, name, locals, nil)
}

// File compiles the statements of a file into a program.
Expand Down Expand Up @@ -1741,31 +1743,33 @@ func (fcomp *fcomp) comprehension(comp *syntax.Comprehension, clauseIndex int) {
}

func (fcomp *fcomp) function(pos syntax.Position, name string, f *syntax.Function) {
// Evalution of the elements of both MAKETUPLEs may fail,
// so record the position.
// Evaluation of the defaults may fail, so record the position.
fcomp.setPos(pos)

// To reduce allocation, we emit a combined tuple
// for the defaults and the freevars.
// The function knows where to split it at run time.

// Generate tuple of parameter defaults. For:
// def f(p1, p2=dp2, p3=dp3, *, k1, k2=dk2, k3, **kwargs)
// the tuple is:
// (dp2, dp3, MANDATORY, dk2, MANDATORY).
n := 0
ndefaults := 0
seenStar := false
for _, param := range f.Params {
switch param := param.(type) {
case *syntax.BinaryExpr:
fcomp.expr(param.Y)
n++
ndefaults++
case *syntax.UnaryExpr:
seenStar = true // * or *args (also **kwargs)
case *syntax.Ident:
if seenStar {
fcomp.emit(MANDATORY)
n++
ndefaults++
}
}
}
fcomp.emit1(MAKETUPLE, uint32(n))

// Capture the cells of the function's
// free variables from the lexical environment.
Expand All @@ -1779,7 +1783,8 @@ func (fcomp *fcomp) function(pos syntax.Position, name string, f *syntax.Functio
fcomp.emit1(LOCAL, uint32(freevar.Index))
}
}
fcomp.emit1(MAKETUPLE, uint32(len(f.FreeVars)))

fcomp.emit1(MAKETUPLE, uint32(ndefaults+len(f.FreeVars)))

funcode := fcomp.pcomp.function(name, pos, f.Body, f.Locals, f.FreeVars)

Expand Down
19 changes: 11 additions & 8 deletions starlark/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func CompiledProgram(in io.Reader) (*Program, error) {
// executes the toplevel code of the specified program,
// and returns a new, unfrozen dictionary of the globals.
func (prog *Program) Init(thread *Thread, predeclared StringDict) (StringDict, error) {
toplevel := makeToplevelFunction(prog.compiled.Toplevel, predeclared)
toplevel := makeToplevelFunction(prog.compiled, predeclared)

_, err := Call(thread, toplevel, nil, nil)

Expand All @@ -361,10 +361,10 @@ func (prog *Program) Init(thread *Thread, predeclared StringDict) (StringDict, e
return toplevel.Globals(), err
}

func makeToplevelFunction(funcode *compile.Funcode, predeclared StringDict) *Function {
func makeToplevelFunction(prog *compile.Program, predeclared StringDict) *Function {
// Create the Starlark value denoted by each program constant c.
constants := make([]Value, len(funcode.Prog.Constants))
for i, c := range funcode.Prog.Constants {
constants := make([]Value, len(prog.Constants))
for i, c := range prog.Constants {
var v Value
switch c := c.(type) {
case int64:
Expand All @@ -382,10 +382,13 @@ func makeToplevelFunction(funcode *compile.Funcode, predeclared StringDict) *Fun
}

return &Function{
funcode: funcode,
predeclared: predeclared,
globals: make([]Value, len(funcode.Prog.Globals)),
constants: constants,
funcode: prog.Toplevel,
module: &module{
program: prog,
predeclared: predeclared,
globals: make([]Value, len(prog.Globals)),
constants: constants,
},
}
}

Expand Down
28 changes: 13 additions & 15 deletions starlark/interp.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ loop:
sp--

case compile.CONSTANT:
stack[sp] = fn.constants[arg]
stack[sp] = fn.module.constants[arg]
sp++

case compile.MAKETUPLE:
Expand All @@ -478,18 +478,16 @@ loop:

case compile.MAKEFUNC:
funcode := f.Prog.Functions[arg]
freevars := stack[sp-1].(Tuple)
defaults := stack[sp-2].(Tuple)
sp -= 2
stack[sp] = &Function{
funcode: funcode,
predeclared: fn.predeclared,
globals: fn.globals,
constants: fn.constants,
defaults: defaults,
freevars: freevars,
tuple := stack[sp-1].(Tuple)
n := len(tuple) - len(funcode.Freevars)
defaults := tuple[:n:n]
freevars := tuple[n:]
stack[sp-1] = &Function{
funcode: funcode,
module: fn.module,
defaults: defaults,
freevars: freevars,
}
sp++

case compile.LOAD:
n := int(arg)
Expand Down Expand Up @@ -533,7 +531,7 @@ loop:
y.(*cell).v = x

case compile.SETGLOBAL:
fn.globals[arg] = stack[sp-1]
fn.module.globals[arg] = stack[sp-1]
sp--

case compile.LOCAL:
Expand All @@ -554,7 +552,7 @@ loop:
stack[sp-1] = x.(*cell).v

case compile.GLOBAL:
x := fn.globals[arg]
x := fn.module.globals[arg]
if x == nil {
err = fmt.Errorf("global variable %s referenced before assignment", f.Prog.Globals[arg].Name)
break loop
Expand All @@ -564,7 +562,7 @@ loop:

case compile.PREDECLARED:
name := f.Prog.Names[arg]
x := fn.predeclared[name]
x := fn.module.predeclared[name]
if x == nil {
err = fmt.Errorf("internal error: predeclared variable %s is uninitialized", name)
break loop
Expand Down
29 changes: 19 additions & 10 deletions starlark/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,15 +570,32 @@ func (*stringIterator) Done() {}
// The initialization behavior of a Starlark module is also represented by a Function.
type Function struct {
funcode *compile.Funcode
module *module
defaults Tuple
freevars Tuple
}

// These fields are shared by all functions in a module.
// A module is the dynamic counterpart to a Program.
// All functions in the same program share a module.
type module struct {
program *compile.Program
predeclared StringDict
globals []Value
constants []Value
}

// makeGlobalDict returns a new, unfrozen StringDict containing all global
// variables so far defined in the module.
func (m *module) makeGlobalDict() StringDict {
r := make(StringDict, len(m.program.Globals))
for i, id := range m.program.Globals {
if v := m.globals[i]; v != nil {
r[id.Name] = v
}
}
return r
}

func (fn *Function) Name() string { return fn.funcode.Name } // "lambda" for anonymous functions
func (fn *Function) Doc() string { return fn.funcode.Doc }
func (fn *Function) Hash() (uint32, error) { return hashString(fn.funcode.Name), nil }
Expand All @@ -589,15 +606,7 @@ func (fn *Function) Truth() Bool { return true }

// Globals returns a new, unfrozen StringDict containing all global
// variables so far defined in the function's module.
func (fn *Function) Globals() StringDict {
m := make(StringDict, len(fn.funcode.Prog.Globals))
for i, id := range fn.funcode.Prog.Globals {
if v := fn.globals[i]; v != nil {
m[id.Name] = v
}
}
return m
}
func (fn *Function) Globals() StringDict { return fn.module.makeGlobalDict() }

func (fn *Function) Position() syntax.Position { return fn.funcode.Pos }
func (fn *Function) NumParams() int { return fn.funcode.NumParams }
Expand Down

0 comments on commit 3ee1685

Please sign in to comment.