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.

Sign up for GitHub

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

Jump to bottom

Cache helm templates for the same inputs #1016

Merged
merged 1 commit into from
Apr 18, 2024
Merged

Cache helm templates for the same inputs #1016

merged 1 commit into from
Apr 18, 2024

Conversation

SimKev2
Copy link
Contributor

@SimKev2 SimKev2 commented Apr 17, 2024

closes #906

Template Caching

Ran some tests with this caching enabled to compare to previous runs

$ time tk export --log-level debug ...
...
real    0m6.126s
user    0m37.292s
sys     0m6.492s

$ time ../../../go/src/github.com/grafana/tanka/tk export --log-level debug ...
...
real    0m5.034s
user    0m33.140s
sys     0m2.881s

To check once more I also tried with --parallel 1 and the times were:

$ time tk export ...
real    0m17.966s
user    0m34.064s
sys     0m3.301s

$ time ../../../go/src/github.com/grafana/tanka/tk export ...
real    0m13.808s
user    0m30.874s
sys     0m1.721s

From the logs I can also verify the cache is being used

{"level":"debug","time":"2024-04-17T11:02:42-05:00","message":"Using cached template for flux2"}

Go bench templateKey

I tried both this current sha256+jsonMarshal combo vs. a fmt.Sprintf of the name + chart + opts and benchmarking shows this current method as the fastest.

pkg: github.com/grafana/tanka/pkg/helm
BenchmarkTemplateKey/emptyData-12          3974046         299.9 ns/op         336 B/op         5 allocs/op
BenchmarkTemplateKey/fullData-12            692072        1681   ns/op        1408 B/op        30 allocs/op
BenchmarkTemplateKeySprintf/emptyData-12   2539329         467.8 ns/op         344 B/op        12 allocs/op
BenchmarkTemplateKeySprintf/fullData-12     523135        2232   ns/op        1728 B/op        48 allocs/op

@CLAassistant
Copy link

CLAassistant commented Apr 17, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@SimKev2 SimKev2 requested a review from julienduchesne April 17, 2024 16:06
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 17, 2024 •
edited
Loading

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-04-18 13:18 UTC

julienduchesne
julienduchesne approved these changes Apr 18, 2024
View reviewed changes
Copy link
Member

@julienduchesne julienduchesne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

pkg/helm/jsonnet_test.go
b.Run(c.name, func(b *testing.B) {
for i := 0; i < b.N; i++ {
// nolint:errcheck
templateKey(c.data.chartName, c.data.chartPath, c.data.opts)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, turns out that json.Marshal really is the fastest option (of the few I tested)

@SimKev2 SimKev2 merged commit f73b937 into main Apr 18, 2024
8 checks passed
@SimKev2 SimKev2 deleted the TANKA-906 branch April 18, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienduchesne julienduchesne julienduchesne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Runs helm repeatedly for the same inputs
3 participants
@SimKev2 @CLAassistant @julienduchesne