Draft
Conversation
Member
Author
|
I'm posting this as a draft since I'm not entirely sure that we should take this change. It is something of a subset of changes that would improve performance in the case of environments with many |
These changes add support for loading environments in parallel. Parallelization is breadth-first, then depth-first. For example, given the following environments: ```yaml imports: # env a - b - c -- imports; # env b - e -- imports: # env c - f - g ``` Environments `b` and `c` would be loaded in parallel, then environment `e` would be loaded, then environments `f` and `g` would be loaded in parallel. This simplifies the detection of cyclic imports and the collection of diagnostics. This improves performance for scenarios that are dominated by environment load time (e.g. import graphs with high degrees of fanout). Local benchmark results: ``` goos: darwin goarch: arm64 pkg: github.com/pulumi/esc/eval cpu: Apple M1 Max BenchmarkEval-10 559 2016141 ns/op 2212786 B/op 18971 allocs/op BenchmarkEval-10 591 1987670 ns/op 2212689 B/op 18970 allocs/op BenchmarkEval-10 614 1977314 ns/op 2212625 B/op 18970 allocs/op BenchmarkEval-10 596 1970144 ns/op 2212937 B/op 18971 allocs/op BenchmarkEval-10 607 1989940 ns/op 2212754 B/op 18970 allocs/op BenchmarkEval-10 628 1934108 ns/op 2212601 B/op 18970 allocs/op BenchmarkEval-10 574 1984031 ns/op 2212765 B/op 18970 allocs/op BenchmarkEval-10 646 1944632 ns/op 2212531 B/op 18970 allocs/op BenchmarkEval-10 631 1954274 ns/op 2212848 B/op 18970 allocs/op BenchmarkEval-10 614 1906139 ns/op 2212780 B/op 18970 allocs/op BenchmarkEvalOpen-10 9 116069278 ns/op 2212690 B/op 18982 allocs/op BenchmarkEvalOpen-10 9 117032264 ns/op 2213604 B/op 18984 allocs/op BenchmarkEvalOpen-10 9 118545838 ns/op 2214613 B/op 18986 allocs/op BenchmarkEvalOpen-10 9 117469657 ns/op 2212854 B/op 18983 allocs/op BenchmarkEvalOpen-10 9 115962963 ns/op 2212275 B/op 18981 allocs/op BenchmarkEvalOpen-10 9 116431745 ns/op 2212888 B/op 18982 allocs/op BenchmarkEvalOpen-10 9 116677569 ns/op 2212040 B/op 18983 allocs/op BenchmarkEvalOpen-10 9 117092157 ns/op 2211416 B/op 18981 allocs/op BenchmarkEvalOpen-10 9 117164181 ns/op 2213941 B/op 18985 allocs/op BenchmarkEvalOpen-10 9 118046199 ns/op 2215542 B/op 18982 allocs/op BenchmarkEvalEnvLoad-10 81 15394315 ns/op 2215076 B/op 18997 allocs/op BenchmarkEvalEnvLoad-10 84 15028452 ns/op 2214531 B/op 18996 allocs/op BenchmarkEvalEnvLoad-10 80 14744255 ns/op 2214963 B/op 18997 allocs/op BenchmarkEvalEnvLoad-10 90 15100905 ns/op 2214574 B/op 18997 allocs/op BenchmarkEvalEnvLoad-10 84 15125639 ns/op 2215519 B/op 18998 allocs/op BenchmarkEvalEnvLoad-10 84 14868314 ns/op 2214583 B/op 18997 allocs/op BenchmarkEvalEnvLoad-10 81 14827542 ns/op 2214839 B/op 18995 allocs/op BenchmarkEvalEnvLoad-10 88 14948501 ns/op 2214839 B/op 18997 allocs/op BenchmarkEvalEnvLoad-10 88 15037255 ns/op 2215109 B/op 18997 allocs/op BenchmarkEvalEnvLoad-10 84 14695667 ns/op 2215256 B/op 18998 allocs/op BenchmarkEvalAll-10 8 127500182 ns/op 2214674 B/op 19009 allocs/op BenchmarkEvalAll-10 8 127734964 ns/op 2215176 B/op 19006 allocs/op BenchmarkEvalAll-10 9 128855694 ns/op 2215416 B/op 19010 allocs/op BenchmarkEvalAll-10 8 128633870 ns/op 2215320 B/op 19009 allocs/op BenchmarkEvalAll-10 9 129122597 ns/op 2215123 B/op 19007 allocs/op BenchmarkEvalAll-10 8 129013708 ns/op 2213784 B/op 19004 allocs/op BenchmarkEvalAll-10 9 127839343 ns/op 2214552 B/op 19007 allocs/op BenchmarkEvalAll-10 9 129212421 ns/op 2214800 B/op 19006 allocs/op BenchmarkEvalAll-10 9 128671162 ns/op 2215864 B/op 19009 allocs/op BenchmarkEvalAll-10 9 127706639 ns/op 2214455 B/op 19006 allocs/op ```
8f171f8 to
c221caf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These changes add support for loading environments in parallel.
Parallelization is breadth-first, then depth-first. For example, given the following environments:
Environments
bandcwould be loaded in parallel, then environmentewould be loaded, then environmentsfandgwould be loaded in parallel. This simplifies the detection of cyclic imports and the collection of diagnostics.This improves performance for scenarios that are dominated by environment load time (e.g. import graphs with high degrees of fanout).
This is arguably a breaking change: prior to these changes, environments with cyclic imports would still be evaluated to the greatest extent possible. With these changes, the evaluator will terminate after attempting to load the import graph but before performing after evaluation in the case of an import cycle. I think that this break is worth the improvement.
Local benchmark results: