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

chore: improve /healthz endpoint performance #2014

Merged
merged 3 commits into from
Jan 23, 2022

Conversation

inkel
Copy link
Contributor

@inkel inkel commented Jan 21, 2022

👋 team. First and foremost: thank you for open sourcing and maintaining this project, it's been really useful. Today while working on an unrelated issue I started to investigate the /healthz endpoint in Atlantis and found that it could be improved in both speed and memory allocations.

The first thing I did was adding a benchmark to have a baseline for comparing my changes:

goos: darwin
goarch: amd64
pkg: github.com/runatlantis/atlantis/server
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkHealthz-12       100000               531.0 ns/op           112 B/op          4 allocs/op
PASS
ok      github.com/runatlantis/atlantis/server  0.370s

Then I implemented my improvements. I noticed that the response body is always the following:

{
  "status": "ok"
}

However, I noticed the body was built by defining an inline struct and calling json.MarshalIndent on it, checking if there was a marshaling error which should never happen, so I removed all of that code and simply define a variable with the expected body. The benchmarks results already show improvements:

goos: darwin
goarch: amd64
pkg: github.com/runatlantis/atlantis/server
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkHealthz-12       100000                94.31 ns/op           40 B/op          2 allocs/op
PASS
ok      github.com/runatlantis/atlantis/server  1.431s

Lastly, I realized there was no need to always initialize this variable, so I moved it to a private package variable and ran the benchmarks again:

goos: darwin
goarch: amd64
pkg: github.com/runatlantis/atlantis/server
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkHealthz-12       100000                60.20 ns/op           16 B/op          1 allocs/op
PASS
ok      github.com/runatlantis/atlantis/server  0.336s

Most of all of the allocations were removed and the speedup is ~90%! In all steps I actually generated more benchmarks results using the following command:

go test -v -{run,bench}=Healthz -benchmem -benchtime=100000x -count=100 ./server/

With this I was able to later run benchstat to compare the results:

name        old time/op    new time/op    delta
Healthz-12     444ns ± 7%      52ns ±25%  -88.33%  (p=0.000 n=84+100)

name        old alloc/op   new alloc/op   delta
Healthz-12      112B ± 0%       16B ± 0%  -85.71%  (p=0.000 n=100+100)

name        old allocs/op  new allocs/op  delta
Healthz-12      4.00 ± 0%      1.00 ± 0%  -75.00%  (p=0.000 n=100+100)

Anyway, I hope you find this useful and worthy of contribution! 😸

This benchmark will measure not only speed but also how much memory
each request to /healthz allocate. I'm adding the benchmark to have a
baseline to compare.

Signed-off-by: Leandro López (inkel) <[email protected]>
We know it will always be this value, there's no need to import and
call json.MarshalIndent.

Signed-off-by: Leandro López (inkel) <[email protected]>
Signed-off-by: Leandro López (inkel) <[email protected]>
@inkel inkel requested a review from a team as a code owner January 21, 2022 18:24
@chenrui333 chenrui333 changed the title Improve /healthz endpoint performance chore: improve /healthz endpoint performance Jan 21, 2022
@chenrui333 chenrui333 merged commit ca28b57 into runatlantis:master Jan 23, 2022
krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
* Add benchmark for /healthz endpoint

This benchmark will measure not only speed but also how much memory
each request to /healthz allocate. I'm adding the benchmark to have a
baseline to compare.

Signed-off-by: Leandro López (inkel) <[email protected]>

* Hardcode /healthz body into a variable

We know it will always be this value, there's no need to import and
call json.MarshalIndent.

Signed-off-by: Leandro López (inkel) <[email protected]>

* Initialize /healthz body only once

Signed-off-by: Leandro López (inkel) <[email protected]>
@inkel inkel deleted the inkel/healthz-performance branch January 2, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants