internal/core/adt: cache containsDefID results

Judson's project in Unity was spending about 70% of CPU time
in containsDefIDRec, because the evaluator made over 700 million calls
to containsDefID, where over half were repeated in terms of the
node and child pair of defID parameters.

With this change, the wall time of exporting Judson's config
drops from ~22s to ~15s, and containsDefIDRec drops from taking
70% of CPU time to just over 50%.

Note that there is some added overhead due to the caching;
out of the new 20s of CPU time spent, about 1.2s is spent reading
and writing to the cache map. However, this seems well worthwhile,
given that we save 7s even with this overhead.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: If1bb69febb515f03e97f0f82c72ee1e784b1adef
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1224497
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

Daniel Martí 8 months ago 992c1cd6 8dce6dc7

+41 -21

2 changed files

expand all

internal

core

adt

eval.go

typocheck.go

+22 -19

internal/core/adt/eval.go

··· 305 305 cyclicConjuncts []cyclicConjunct 306 306 307 307 // These fields are used to track type checking. 308 - reqDefIDs []refInfo 309 - replaceIDs []replaceID 310 - conjunctInfo []conjunctInfo 311 - reqSets reqSets 308 + reqDefIDs []refInfo 309 + replaceIDs []replaceID 310 + conjunctInfo []conjunctInfo 311 + reqSets reqSets 312 + containsDefIDCache map[[2]defID]bool // cache for containsDefID results 312 313 313 314 // Checks is a list of conjuncts, as we need to preserve the context in 314 315 // which it was evaluated. The conjunct is always a validator (and thus ··· 492 493 nodeContextState: nodeContextState{ 493 494 kind: TopKind, 494 495 }, 495 - toFree: n.toFree[:0], 496 - arcMap: n.arcMap[:0], 497 - cyclicConjuncts: n.cyclicConjuncts[:0], 498 - notify: n.notify[:0], 499 - sharedIDs: n.sharedIDs[:0], 500 - checks: n.checks[:0], 501 - postChecks: n.postChecks[:0], 502 - reqDefIDs: n.reqDefIDs[:0], 503 - replaceIDs: n.replaceIDs[:0], 504 - conjunctInfo: n.conjunctInfo[:0], 505 - reqSets: n.reqSets[:0], 506 - disjunctions: n.disjunctions[:0], 507 - disjunctErrs: n.disjunctErrs[:0], 508 - userErrs: n.userErrs[:0], 509 - disjuncts: n.disjuncts[:0], 496 + toFree: n.toFree[:0], 497 + arcMap: n.arcMap[:0], 498 + cyclicConjuncts: n.cyclicConjuncts[:0], 499 + notify: n.notify[:0], 500 + sharedIDs: n.sharedIDs[:0], 501 + checks: n.checks[:0], 502 + postChecks: n.postChecks[:0], 503 + reqDefIDs: n.reqDefIDs[:0], 504 + replaceIDs: n.replaceIDs[:0], 505 + conjunctInfo: n.conjunctInfo[:0], 506 + reqSets: n.reqSets[:0], 507 + disjunctions: n.disjunctions[:0], 508 + disjunctErrs: n.disjunctErrs[:0], 509 + userErrs: n.userErrs[:0], 510 + disjuncts: n.disjuncts[:0], 511 + containsDefIDCache: n.containsDefIDCache, // cleared below 510 512 } 513 + clear(n.containsDefIDCache) 511 514 n.scheduler.clear() 512 515 } else { 513 516 c.stats.Allocs++

+19 -2

internal/core/adt/typocheck.go

··· 708 708 } 709 709 710 710 func (n *nodeContext) containsDefID(node, child defID) bool { 711 - // TODO(perf): cache result 712 711 // TODO(perf): we could keep track of the minimum defID that could map so 713 712 // that we can use this to bail out early. 714 713 c := n.ctx 714 + 715 + key := [2]defID{node, child} 716 + if result, ok := n.containsDefIDCache[key]; ok { 717 + return result 718 + } 719 + 715 720 c.redirectsBuf = c.redirectsBuf[:0] 716 721 for p := n; p != nil; p = p.node.Parent.state { 717 722 if p.opID != n.opID { ··· 727 732 c.stats.MaxRedirect = int64(len(c.redirectsBuf)) 728 733 } 729 734 730 - return n.containsDefIDRec(node, child, child) 735 + result := n.containsDefIDRec(node, child, child) 736 + 737 + // Caching in [nodeContext.containsDefIDCache] adds overhead; 738 + // only do it if we estimate that [nodeContext.containsDefIDRec] 739 + // is doing significant work by looking at the number of replaceIDs. 740 + if len(c.redirectsBuf) > 15 { 741 + if n.containsDefIDCache == nil { 742 + n.containsDefIDCache = make(map[[2]defID]bool) 743 + } 744 + n.containsDefIDCache[key] = result 745 + } 746 + 747 + return result 731 748 } 732 749 733 750 func (n *nodeContext) containsDefIDRec(node, child, start defID) bool {

Configure Feed

Configure Feed