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

Render marshalSJON/Raw/CSV preallocated bytes buffer #729

Merged
merged 2 commits into from
Oct 21, 2022

Conversation

msaf1980
Copy link
Collaborator

$ go test -benchmem -run=^$ -bench ^Benchmark github.com/go-graphite/carbonapi/expr/types | tee marshalJSON2.txt 

$ benchcmp marshalJSON1.txt marshalJSON2.txt 
benchmark                      old ns/op     new ns/op     delta
BenchmarkMarshalJSON-6         9297315       7406833       -20.33%
BenchmarkMarshalJSONLong-6     135895934     121057103     -10.92%
BenchmarkMarshalRaw-6          5717379       5622104       -1.67%
BenchmarkMarshalRawLong-6      94664746      90235233      -4.68%
BenchmarkMarshalCSV-6          38516100      19187483      -50.18%
BenchmarkMarshalCSVLong-6      587776864     320056366     -45.55%

benchmark                      old allocs     new allocs     delta
BenchmarkMarshalJSON-6         39             5              -87.18%
BenchmarkMarshalJSONLong-6     89             43             -51.69%
BenchmarkMarshalRaw-6          29             1              -96.55%
BenchmarkMarshalRawLong-6      42             1              -97.62%
BenchmarkMarshalCSV-6          110037         3              -100.00%
BenchmarkMarshalCSVLong-6      1820049        6              -100.00%

benchmark                      old bytes     new bytes     delta
BenchmarkMarshalJSON-6         8401270       2569729       -69.41%
BenchmarkMarshalJSONLong-6     130283752     27232828      -79.10%
BenchmarkMarshalRaw-6          1981197       2564105       +29.42%
BenchmarkMarshalRawLong-6      41704200      25608206      -38.60%
BenchmarkMarshalCSV-6          23749513      9773060       -58.85%
BenchmarkMarshalCSVLong-6      358308832     288342088     -19.53%

@msaf1980 msaf1980 marked this pull request as draft October 20, 2022 15:57
@msaf1980
Copy link
Collaborator Author

Also may be better direct write to http.ResponseWriter instead of use bytes buffer. It's will be require less memory for large metrrics set.

@Civil
Copy link
Member

Civil commented Oct 20, 2022

Yeah, that should work as well. I'm not sure it won't have any complications on the behavior, but might improve time to first byte as well for consumers.

@msaf1980
Copy link
Collaborator Author

Yeah, that should work as well. I'm not sure it won't have any complications on the behavior, but might improve time to first byte as well for consumers.

Also I'm not have experience with this. It can produce chunked response (on large metrics set) and need more tests, that no bugs in http clients, grafana, etc.
May be better merge current, and research some later ?

@msaf1980 msaf1980 marked this pull request as ready for review October 21, 2022 05:50
@Civil Civil merged commit 396f5b0 into go-graphite:main Oct 21, 2022
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.

None yet

2 participants