-
Notifications
You must be signed in to change notification settings - Fork 312
evalv3 performance regression which disappears with CUE_DEBUG=opendef #3881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Increasing the
So I believe this TODO is to blame:
|
LogEval is zero by default, and its implementation via Indentf has a non-zero cost which I spotted via pprof, so avoid calling the funcs entirely in all the call sites. Some already skipped via an early LogEval check, but others did not. Benchmarking the code from https://cuelang.org/issue/3881 via benchcmd -n 8 CueVetIssue3881 cue vet -c f.cue: │ old │ new │ │ sec/op │ sec/op vs base │ CueVetIssue3881 507.4m ± 0% 496.0m ± 1% -2.24% (p=0.000 n=8) │ old │ new │ │ user-sec/op │ user-sec/op vs base │ CueVetIssue3881 571.9m ± 1% 560.0m ± 1% -2.08% (p=0.003 n=8) │ old │ new │ │ sys-sec/op │ sys-sec/op vs base │ CueVetIssue3881 12.159m ± 51% 9.510m ± 58% ~ (p=0.234 n=8) │ old │ new │ │ peak-RSS-bytes │ peak-RSS-bytes vs base │ CueVetIssue3881 47.95Mi ± 3% 49.42Mi ± 4% ~ (p=0.857 n=8) Updates #3881. Signed-off-by: Daniel Martí <mvdan@mvdan.cc> Change-Id: I8ec0215239e97f6e8aeaeb50196b42910d91a815 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1213054 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Issue #3881 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I8dea8779b3fe23eaaea688f72a62749928eeb4db Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1213606 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Matthew Sackman <matthew@cue.works>
This should reduce the amount of work done by a factor of the depth of a tree. This doesn't change the essence of the algorithm. Issue #3881 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I2ad81c04b28b64b9736463489039806a4493e5bd Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1213607 Reviewed-by: Matthew Sackman <matthew@cue.works> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Below are the perf numbers from #3879 (
FYI @joelMuehlena we made some progress here, but more is clearly needed, so we're leaving this issue open for now. |
As of e058014:
You can see that evalv2 runs in about 40ms, evalv3 runs in 650ms (more than a 10x slow-down), but evalv3 with opendef runs in 40ms once again.
Distilled from #3879.
The text was updated successfully, but these errors were encountered: