Skip to content
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

Improve performance of #Lineage and BindLineage #137

Closed
sdboyer opened this issue May 6, 2023 · 5 comments · Fixed by #173
Closed

Improve performance of #Lineage and BindLineage #137

sdboyer opened this issue May 6, 2023 · 5 comments · Fixed by #173
Assignees
Labels
enhancement New feature or request performance prio/high High priority readiness/minimum Requisite for achieving a minimum readiness

Comments

@sdboyer
Copy link
Contributor

sdboyer commented May 6, 2023

Performance of lineage loading and binding declined significantly with lineage flattening (#82). Preliminary profiling (I've attached a - lol - screenshot) indicates that most of the problem occurs during calls to cue.Context.BuildInstance(), which indicates that it's the CUE evaluator struggling with working out all the new internal structures within the native CUE definition of #Lineage.

Image

(Note that the profiler has erroneously put cue.Value.Unify under load.Instances - the main cost there is incurred during the BuildInstance() call).)

My guess is that we have some low-hanging fruit here related to some of the new disjunctions introduced. I suspect we can express the same constraints in a different way and improve performance here ocnsiderably.

@sdboyer
Copy link
Contributor Author

sdboyer commented May 24, 2023

We'll definitely want to add some benchmark tests as part of this. And i see that https://pkg.go.dev/golang.org/x/perf/cmd/benchstat exists, which could be useful in helping us track progress against this over time.

@joanlopez
Copy link
Contributor

joanlopez commented May 29, 2023

We'll definitely want to add some benchmark tests as part of this. And i see that https://pkg.go.dev/golang.org/x/perf/cmd/benchstat exists, which could be useful in helping us track progress against this over time.

Related with that, I've just discovered gobenchdata, and it looks really nice!

Example demo: https://gobenchdata.bobheadxi.dev/

@joanlopez joanlopez removed their assignment May 29, 2023
@sdboyer
Copy link
Contributor Author

sdboyer commented Jun 2, 2023

oh amazing, that looks like the path of least resistance

@joanlopez
Copy link
Contributor

Some initial insights from BindLineage: looks like most of the time is spent on this line.

@sdboyer
Copy link
Contributor Author

sdboyer commented Jun 7, 2023

My guess is that AppendSplit is still just a symptom of an underlying structure that the evaluator is tripping over - it's the call to cue.Value.Expr() that's expensive, as this bit of the profile shows:

Screenshot 2023-06-07 at 5 33 52 AM

So it's still looking like something in the pure CUE definitions that's causing the basic churn. But having the different benchmark cases does also help us differentiate a bit...lots of good info to go through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request performance prio/high High priority readiness/minimum Requisite for achieving a minimum readiness
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants