-
Notifications
You must be signed in to change notification settings - Fork 169
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
perf(export): only call FindEnvs once #553
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
@@ -92,6 +80,7 @@ type parallelOut struct { | |||
|
|||
func parallelWorker(jobsCh <-chan parallelJob, outCh chan parallelOut) { | |||
for job := range jobsCh { | |||
log.Printf("Loading %s from %s", job.opts.Name, job.path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Supposed to be here? Sounds quite verbose imo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it intentionally. It gives a bit of feedback to the user as opposed to no feedback right now. I'm happy to remove or accept other suggestions to give a sense of progress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. I'm not totally familiar, does this codepath affect any other actions than export?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I know off.
@@ -92,6 +80,7 @@ type parallelOut struct { | |||
|
|||
func parallelWorker(jobsCh <-chan parallelJob, outCh chan parallelOut) { | |||
for job := range jobsCh { | |||
log.Printf("Loading %s from %s", job.opts.Name, job.path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. I'm not totally familiar, does this codepath affect any other actions than export?
On the internal Grafana Labs jsonnet code base, this can improve the performance of the export significantly. Tests on my local
machine went from 2m08s to 1m32s.