Lazy evaluation recursed once per link of a chain of formulas: a chain through every cell of a full sheet overflowed the 1 GB goroutine stack and killed the program. Evaluation now counts its nesting in cells and operators; a cell reached past 65536 levels is put off, the evaluation in progress is abandoned and retried once that cell is done. Values are as before, a cycle longer than the limit is still a cycle, and the stack stays under about 30 MB.
- commit
- 7140572955a6c0bba351e82b6daf3fe310049d77
- parent
- 00faa1f90ae8dc54d621122e4bcdefa6cfb49356
- author
- Dami <dami.etoile@gmail.com>
| M | internal/ | 13 | +++ |
| A | internal/ | 153 | ++++++++++++++++++++++++++++++ |
| A | internal/ | 103 | ++++++++++++++++++++ |
| M | internal/ | 71 | +-------------- |
| M | internal/ | 9 | +- |
@@ -118,6 +118,19 @@ func eval(n Node, get lookup) Value {
118118 return get.cell(n.Sheet, n.Rect.From)119119 }120120 return ErrValue121+ case formula.Unary, decUnary, formula.Binary, decBinary, formula.Call:122+ get.w.depth++123+ v := evalNested(n, get)124+ get.w.depth--125+ return v126+ }127+ return ErrValue128+}129+130+// evalNested computes an operator or a call, which evaluate their131+// operands: eval counts the nesting around it (see evaluate.go).132+func evalNested(n Node, get lookup) Value {133+ switch n := n.(type) {121134 case formula.Unary:122135 return evalUnary(n, get, false)123136 case decUnary:@@ -0,0 +1,153 @@
1+package sheet2+3+import (4+ "strings"5+6+ "github.com/FelineStateMachine/012/internal/formula"7+)8+9+// Evaluation is lazy and recursive: a formula reading a dirty cell10+// evaluates it first, so a chain of formulas (each row adding to the row11+// above) recurses once per link. The parser caps how deeply one formula12+// nests (formula.MaxDepth), but a chain can run through every cell of a13+// sheet, two million links, which would take more than the goroutine's14+// stack. So evaluation counts how deep it is (w.depth: cells and15+// operators, nested), and a cell it reaches past maxEvalDepth is put off:16+// the evaluation in progress is abandoned (its cells go back to dirty),17+// the cell is evaluated on its own, from the top of the stack, and then18+// what was abandoned is tried again, finding it done. Values come out as19+// if evaluated in one go; a long chain costs about twice the work.20+//21+// Cells put off are "deferred" until they're evaluated: each depends on22+// the one put off after it, so reaching one again is a cycle, as reaching23+// a cell being evaluated is.24+25+// maxEvalDepth bounds evaluation's nesting, in cells and operators: about26+// 30 MB of stack. A var so tests can lower it.27+var maxEvalDepth = 1 << 1628+29+// tooDeep abandons an evaluation that went past maxEvalDepth.30+type tooDeep struct{}31+32+// evaluator is the state of one evaluate.33+type evaluator struct {34+ w *Workbook35+ stack []loc // cells being evaluated, outermost first36+ pending []loc // cells put off, each needed by the one before37+}38+39+// evaluate computes the cells marked dirty in each sheet's calc. Cells40+// are evaluated lazily in dependency order: reading a dirty cell41+// evaluates it first. A cell that is reached again while it is still42+// being evaluated is part of a cycle and becomes ERR. The state lives on43+// the sheets, keyed by address as on one sheet, and each sheet's lookup44+// is made once, so evaluating a formula allocates nothing for sheets.45+func (w *Workbook) evaluate() {46+ w.Circular = false47+ e := &evaluator{w: w}48+ for _, s := range w.sheets {49+ s.version++50+ s.hidden.valid = false // values may have changed what the filter hides51+ s.calcGet = w.lookupOn(s, e.compute)52+ s.calcFmt = w.formatFrom(s)53+ }54+ for _, s := range w.sheets {55+ for a, st := range s.calc {56+ if st == dirty {57+ e.settle(loc{s, a})58+ }59+ }60+ }61+ for _, s := range w.sheets {62+ s.calc, s.calcGet, s.calcFmt = nil, nil, nil63+ }64+}65+66+// compute returns the value of the cell at a, evaluating it first if it's67+// dirty. Every cell a formula reads comes through here, so it looks each68+// map up once: a SUM over 8192 cells makes 8192 calls.69+func (e *evaluator) compute(s *Sheet, a Addr) Value {70+ w := e.w71+ c := s.cells.get(a)72+ switch st := s.calc[a]; {73+ case c == nil:74+ return Value{}75+ case st == visiting, st == deferred:76+ w.Circular = true77+ return ErrRef78+ case st != dirty:79+ return c.Value80+ case c.derived: // a pivot's result, set when the pivot was computed81+ s.calc[a] = done82+ return c.Value83+ case w.depth >= maxEvalDepth:84+ s.calc[a] = deferred85+ e.pending = append(e.pending, loc{s, a})86+ panic(tooDeep{})87+ }88+ s.calc[a] = visiting89+ e.stack = append(e.stack, loc{s, a})90+ w.depth++91+ c.auto = Format{}92+ switch {93+ case c.Input == "":94+ c.Value = Value{}95+ case c.expr == nil && c.Format.Kind == FmtText:96+ c.Value = Value{Kind: Text, Str: c.Input}97+ case c.expr == nil:98+ c.Value = Value{Kind: Text, Str: strings.TrimPrefix(c.Input, "'")}99+ default:100+ expr := s.bound(c)101+ outer := w.evaluating102+ w.evaluating = loc{s, a}103+ c.Value = eval(w.arith(expr), s.calcGet)104+ w.evaluating = outer105+ if _, lit := expr.(formula.Num); !lit {106+ c.auto = inferFormat(expr, s.calcFmt)107+ }108+ }109+ w.depth--110+ e.stack = e.stack[:len(e.stack)-1]111+ s.calc[a] = done112+ s.cells.changed(a)113+ return c.Value114+}115+116+// settle evaluates the dirty cell at l, and every cell put off on the117+// way, deepest first.118+func (e *evaluator) settle(l loc) {119+ e.pending = append(e.pending[:0], l)120+ for len(e.pending) > 0 {121+ top := e.pending[len(e.pending)-1]122+ if top.s.calc[top.a] == deferred {123+ top.s.calc[top.a] = dirty124+ }125+ if e.try(top) {126+ e.pending = e.pending[:len(e.pending)-1]127+ }128+ }129+}130+131+// try evaluates the cell at l, reporting false if it went too deep and132+// put off a cell, having undone what it had started.133+func (e *evaluator) try(l loc) (finished bool) {134+ defer func() {135+ if r := recover(); r != nil {136+ if _, deep := r.(tooDeep); !deep {137+ panic(r)138+ }139+ for _, v := range e.stack {140+ v.s.calc[v.a] = dirty141+ }142+ for _, p := range e.pending {143+ p.s.calc[p.a] = deferred144+ }145+ e.stack = e.stack[:0]146+ e.w.depth = 0147+ e.w.evaluating = loc{}148+ finished = false149+ }150+ }()151+ e.compute(l.s, l.a)152+ return true153+}@@ -0,0 +1,103 @@
1+package sheet2+3+import (4+ "fmt"5+ "os"6+ "runtime/debug"7+ "testing"8+)9+10+// withEvalDepth lowers maxEvalDepth for a test.11+func withEvalDepth(t *testing.T, n int) {12+ t.Helper()13+ old := maxEvalDepth14+ maxEvalDepth = n15+ t.Cleanup(func() { maxEvalDepth = old })16+}17+18+// chain fills column col, rows 1 to n, with =A(row-1)+1 below a 1, so19+// the last is n.20+func chain(s *Sheet, col, n int) {21+ for row := range n {22+ in := "1"23+ if row > 0 {24+ in = "=" + Addr{Col: col, Row: row - 1}.String() + "+1"25+ }26+ s.Load(Addr{Col: col, Row: row}, in, Format{}, Style{})27+ }28+}29+30+// Evaluation past maxEvalDepth puts cells off and comes back to them:31+// values come out as if evaluated in one go.32+func TestDeepChains(t *testing.T) {33+ withEvalDepth(t, 10)34+ s := New()35+ chain(s, 0, 1000)36+ // Deep formulas in a chain: each cell nests 8 levels.37+ for row := range 300 {38+ in := "=1"39+ if row > 0 {40+ in = "=((((((((" + Addr{Col: 1, Row: row - 1}.String() + "+1))))))))"41+ }42+ s.Load(Addr{Col: 1, Row: row}, in, Format{}, Style{})43+ }44+ s.RecalcAll()45+ if got := s.Value(Addr{Row: 999}); got != num(1000) {46+ t.Errorf("A1000 = %+v", got)47+ }48+ if got := s.Value(Addr{Col: 1, Row: 299}); got != num(300) {49+ t.Errorf("B300 = %+v", got)50+ }51+ if s.Book().Circular {52+ t.Error("a chain was taken for a cycle")53+ }54+ // An edit at the top recalculates the chain the same way.55+ s.Set(at("A1"), "5")56+ if got := s.Value(Addr{Row: 999}); got != num(1004) {57+ t.Errorf("after an edit, A1000 = %+v", got)58+ }59+}60+61+// A cycle longer than maxEvalDepth is still a cycle.62+func TestDeepCycle(t *testing.T) {63+ withEvalDepth(t, 10)64+ s := New()65+ chain(s, 0, 500)66+ s.Set(at("A1"), "=A500+1")67+ if !s.Book().Circular {68+ t.Error("a cycle of 500 cells wasn't found")69+ }70+ if got := s.Value(at("A250")); got.Kind != Error {71+ t.Errorf("A250, in the cycle, = %+v", got)72+ }73+ s.Set(at("A1"), "1")74+ if s.Book().Circular || s.Value(at("A500")) != num(500) {75+ t.Errorf("after breaking the cycle: circular %v, A500 = %+v", s.Book().Circular, s.Value(at("A500")))76+ }77+}78+79+// A chain through every cell of a full sheet fits in a bounded stack.80+// Slow (2 M cells, 1 GB): set SHEET_FULL_CHAIN=1 to run it.81+func TestFullSheetChain(t *testing.T) {82+ if os.Getenv("SHEET_FULL_CHAIN") == "" {83+ t.Skip("set SHEET_FULL_CHAIN=1 to evaluate a chain through all 2 M cells")84+ }85+ defer debug.SetMaxStack(debug.SetMaxStack(64 << 20))86+ s := New()87+ for col := range MaxCols {88+ for row := range MaxRows {89+ in := "1"90+ switch {91+ case row > 0:92+ in = "=" + Addr{Col: col, Row: row - 1}.String() + "+1"93+ case col > 0:94+ in = "=" + Addr{Col: col - 1, Row: MaxRows - 1}.String() + "+1"95+ }96+ s.Load(Addr{Col: col, Row: row}, in, Format{}, Style{})97+ }98+ }99+ s.RecalcAll()100+ if got, want := s.Value(Addr{Col: MaxCols - 1, Row: MaxRows - 1}), num(MaxCols*MaxRows); got != want {101+ t.Errorf("IV8192 = %+v, want %v", got, fmt.Sprint(want.Num))102+ }103+}@@ -1,10 +1,6 @@
11 package sheet22 3-import (4- "strings"5-6- "github.com/FelineStateMachine/012/internal/formula"7-)3+import "github.com/FelineStateMachine/012/internal/formula"84 95 // Recalculation: a change marks the changed cells and everything that106 // transitively reads them dirty, on any sheet, then evaluates the dirty@@ -46,6 +42,7 @@ const (
4642 dirty = iota + 14743 visiting4844 done45+ deferred // put off: evaluation went too deep, see evaluate.go4946 )5047 5148 // recalc recomputes the changed cells, volatile formulas, and everything@@ -168,70 +165,6 @@ func (c *Cell) rangeHas(a Addr) bool {
168165 return false169166 }170167 171-// evaluate computes the cells marked dirty in each sheet's calc. Cells172-// are evaluated lazily in dependency order: reading a dirty cell173-// evaluates it first. A cell that is reached again while it is still174-// being evaluated is part of a cycle and becomes ERR. The state lives on175-// the sheets, keyed by address as on one sheet, and each sheet's lookup176-// is made once, so evaluating a formula allocates nothing for sheets.177-func (w *Workbook) evaluate() {178- w.Circular = false179- var compute func(s *Sheet, a Addr) Value180- compute = func(s *Sheet, a Addr) Value {181- // Every cell a formula reads comes through here, so it looks182- // each map up once: a SUM over 8192 cells makes 8192 calls.183- c := s.cells.get(a)184- switch st := s.calc[a]; {185- case c == nil:186- return Value{}187- case st == visiting:188- w.Circular = true189- return ErrRef190- case st != dirty:191- return c.Value192- case c.derived: // a pivot's result, set when the pivot was computed193- s.calc[a] = done194- return c.Value195- }196- s.calc[a] = visiting197- c.auto = Format{}198- switch {199- case c.Input == "":200- c.Value = Value{}201- case c.expr == nil && c.Format.Kind == FmtText:202- c.Value = Value{Kind: Text, Str: c.Input}203- case c.expr == nil:204- c.Value = Value{Kind: Text, Str: strings.TrimPrefix(c.Input, "'")}205- default:206- expr := s.bound(c)207- outer := w.evaluating208- w.evaluating = loc{s, a}209- c.Value = eval(w.arith(expr), s.calcGet)210- w.evaluating = outer211- if _, lit := expr.(formula.Num); !lit {212- c.auto = inferFormat(expr, s.calcFmt)213- }214- }215- s.calc[a] = done216- s.cells.changed(a)217- return c.Value218- }219- for _, s := range w.sheets {220- s.version++221- s.hidden.valid = false // values may have changed what the filter hides222- s.calcGet = w.lookupOn(s, compute)223- s.calcFmt = w.formatFrom(s)224- }225- for _, s := range w.sheets {226- for a := range s.calc {227- compute(s, a)228- }229- }230- for _, s := range w.sheets {231- s.calc, s.calcGet, s.calcFmt = nil, nil, nil232- }233-}234-235168 // crossReads reports whether the formula at u reads the cell l through a236169 // reference that names l's sheet.237170 func (w *Workbook) crossReads(u, l loc) bool {@@ -45,9 +45,12 @@ type Workbook struct {
4545 // being evaluated, which a question is asked for (see remote.go).4646 waiting map[string]map[loc]struct{}4747 evaluating loc48- hist history // undo and redo, see history.go49- active int // the sheet last shown, saved in the file50- decimal bool // decimal arithmetic, see decimal.go48+ // depth counts the cells and operators being evaluated, nested; see49+ // evaluate.go.50+ depth int51+ hist history // undo and redo, see history.go52+ active int // the sheet last shown, saved in the file53+ decimal bool // decimal arithmetic, see decimal.go5154 5255 macros []Macro // see macros.go5356 macroOrigin string