Improve plan summary performance

[inkel]

what

At Grafana Labs we have been using Atlantis for several months, and a typical PR could affect over 10 to 20 different projects; these PRs happen several times a day, meaning Atlantis reports back plan summaries very often.

While looking at how to override the templates to better accommodate them to our needs, I've found that the *PlanSuccess.Summary method could be made much faster by simply extracting the compilation of the regexes outside the method instead of compiling them every time.

why

As described above we rely heavily on Atlantis giving us faster feedback. By improving the performance in speed and memory allocations the feedback cycle will be faster and also we will be reducing our carbon footprint.

tests

I first introduced a benchmark for this method, and ran the tests and benchmark locally to take a baseline:

$ go test -bench=. ./server/events/command
goos: darwin
goarch: arm64
pkg: github.com/runatlantis/atlantis/server/events/command
BenchmarkPlanSuccess_Summary/empty_summary,_no_matches-10                  82564             12415 ns/op           27352 B/op        114 allocs/op
BenchmarkPlanSuccess_Summary/changes-10                                   167904              7148 ns/op           14309 B/op         69 allocs/op
BenchmarkPlanSuccess_Summary/no_changes-10                                 92410             12984 ns/op           27705 B/op        114 allocs/op
BenchmarkPlanSuccess_Summary/changes_outside_Terraform-10                  89256             13505 ns/op           27882 B/op        117 allocs/op
BenchmarkPlanSuccess_Summary/changes_and_changes_outside-10               159199              7527 ns/op           14493 B/op         72 allocs/op
PASS
ok      github.com/runatlantis/atlantis/server/events/command   7.536s

After introducing my changes, I've ran the benchmarks and the tests again, and got this:

$ go test -bench=. ./server/events/command 
goos: darwin
goarch: arm64
pkg: github.com/runatlantis/atlantis/server/events/command
BenchmarkPlanSuccess_Summary/changes_outside_Terraform-10                1412385               820.7 ns/op           193 B/op          3 allocs/op
BenchmarkPlanSuccess_Summary/changes_and_changes_outside-10              1587134               757.8 ns/op           177 B/op          3 allocs/op
BenchmarkPlanSuccess_Summary/empty_summary,_no_matches-10               90337636                13.15 ns/op            0 B/op          0 allocs/op
BenchmarkPlanSuccess_Summary/changes-10                                  3303454               363.9 ns/op             0 B/op          0 allocs/op
BenchmarkPlanSuccess_Summary/no_changes-10                               3318954               361.5 ns/op             0 B/op          0 allocs/op
PASS
ok      github.com/runatlantis/atlantis/server/events/command   9.102s

To have a better comparison I've ran all benchmarks with 200,000 iterations, 10 times each, and got the following result using benchstat:

$ benchstat orig.txt improv.txt
name                                                old time/op    new time/op    delta
PlanSuccess_Summary/empty_summary,_no_matches-10      12.6µs ± 2%     0.0µs ± 1%   -99.90%  (p=0.000 n=10+9)
PlanSuccess_Summary/changes-10                        7.19µs ± 0%    0.36µs ± 2%   -94.96%  (p=0.000 n=8+9)
PlanSuccess_Summary/no_changes-10                     13.2µs ± 2%     0.4µs ± 0%   -97.27%  (p=0.000 n=10+9)
PlanSuccess_Summary/changes_outside_Terraform-10      13.7µs ± 1%     0.8µs ± 0%   -94.02%  (p=0.000 n=9+9)
PlanSuccess_Summary/changes_and_changes_outside-10    7.71µs ± 5%    0.75µs ± 0%   -90.21%  (p=0.000 n=10+8)

name                                                old alloc/op   new alloc/op   delta
PlanSuccess_Summary/empty_summary,_no_matches-10      27.4kB ± 0%     0.0kB       -100.00%  (p=0.000 n=10+10)
PlanSuccess_Summary/changes-10                        14.3kB ± 0%     0.0kB       -100.00%  (p=0.000 n=9+10)
PlanSuccess_Summary/no_changes-10                     27.7kB ± 0%     0.0kB       -100.00%  (p=0.000 n=10+10)
PlanSuccess_Summary/changes_outside_Terraform-10      27.9kB ± 0%     0.2kB ± 0%   -99.31%  (p=0.000 n=10+7)
PlanSuccess_Summary/changes_and_changes_outside-10    14.5kB ± 0%     0.2kB ± 0%   -98.78%  (p=0.000 n=10+8)

name                                                old allocs/op  new allocs/op  delta
PlanSuccess_Summary/empty_summary,_no_matches-10         114 ± 0%         0       -100.00%  (p=0.000 n=10+10)
PlanSuccess_Summary/changes-10                          69.0 ± 0%       0.0       -100.00%  (p=0.000 n=10+10)
PlanSuccess_Summary/no_changes-10                        114 ± 0%         0       -100.00%  (p=0.000 n=10+10)
PlanSuccess_Summary/changes_outside_Terraform-10         117 ± 0%         3 ± 0%   -97.44%  (p=0.000 n=10+10)
PlanSuccess_Summary/changes_and_changes_outside-10      72.0 ± 0%       3.0 ± 0%   -95.83%  (p=0.000 n=10+10)

references

• benchstat

[inkel]

I've noted that PlanSuccess.DiffMarkdownFormattedTerraformOutput also could be improved the same way, however, that one doesn't seem to have a specific test for it, so I haven't changed it yet.

[GenPage]

This is an excellent change and much appreciated with the benchmark testing. 💯

[inkel]

Thanks for the nice words @GenPage!

[jamengual]

@inkel do you think you could add the improvements you mention in PlanSuccess.DiffMarkdownFormattedTerraformOutput in this PR?

and thanks a lot for this PR

[inkel]

@jamengual I sure can! Note that as I said above there are no tests for that specific method, but from what I can gather by reading the code it should be an easy to follow change and nothing should break.

[inkel]

I've found a test for it, it was in the MarkdownRenderer test suite. I'm going to add a benchmark there and the apply the improvements.

[inkel]

@jamengual done. Doing the same changes doesn't reduce the performance as dramatically as we've observed in the summary function, however it does reduce the allocations and memory used:

$ benchstat markdowndiff.orig.txt markdowndiff.inkel.txt 
name                                                                                                                   old time/op    new time/op    delta
RenderProjectResultsWithEnableDiffMarkdownFormat/single_successful_plan_with_diff_markdown_formatted/verbose_true-10      727µs ± 0%     717µs ± 2%   -1.31%  (p=0.002 n=10+10)
RenderProjectResultsWithEnableDiffMarkdownFormat/single_successful_plan_with_diff_markdown_formatted/verbose_false-10     725µs ± 0%     715µs ± 1%   -1.33%  (p=0.000 n=9+10)

name                                                                                                                   old alloc/op   new alloc/op   delta
RenderProjectResultsWithEnableDiffMarkdownFormat/single_successful_plan_with_diff_markdown_formatted/verbose_true-10      171kB ± 0%     153kB ± 0%  -10.60%  (p=0.000 n=10+10)
RenderProjectResultsWithEnableDiffMarkdownFormat/single_successful_plan_with_diff_markdown_formatted/verbose_false-10     171kB ± 0%     153kB ± 0%  -10.60%  (p=0.000 n=10+10)

name                                                                                                                   old allocs/op  new allocs/op  delta
RenderProjectResultsWithEnableDiffMarkdownFormat/single_successful_plan_with_diff_markdown_formatted/verbose_true-10        364 ± 0%       214 ± 0%  -41.29%  (p=0.000 n=10+9)
RenderProjectResultsWithEnableDiffMarkdownFormat/single_successful_plan_with_diff_markdown_formatted/verbose_false-10       358 ± 0%       207 ± 0%  -42.05%  (p=0.000 n=10+10)

[jamengual]

Amazing, and so fast to reply, thanks so much @inkel