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

Add collapse benchmarks #88

Merged
merged 2 commits into from
Mar 12, 2019
Merged

Add collapse benchmarks #88

merged 2 commits into from
Mar 12, 2019

Conversation

Licenser
Copy link
Contributor

No description provided.

@jonhoo
Copy link
Owner

jonhoo commented Mar 12, 2019

Should we run these on CI somehow? I'm a little sad that perf has to go through decoding, but it may be necessary if we don't want to pull in the entire decompressed file.

@Licenser
Copy link
Contributor Author

My experience is I wouldn't, for once they take a long time (3 minutes on my machine) and then especially free CI services are so unreliable performance wise that run to run variance is much to high to give meaningful results.

@jonhoo
Copy link
Owner

jonhoo commented Mar 12, 2019

Good point. I'd like to at least see a section in the README on how to run the benchmarks though!

In fact, thinking about it some more, maybe the perf benchmark should first decompress and write the stacks to a temporary file, and then run the benchmark on that decompressed file.

@Licenser
Copy link
Contributor Author

The benchmark itself is purely run on memory, I wanted to take the IO out of it since we can't affect that part of the performance :).

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@jonhoo
Copy link
Owner

jonhoo commented Mar 12, 2019

Ah, yes, I misread the code and though you were doing the I/O as part of the benchmark! Ignore me 😅

@codecov-io
Copy link

Codecov Report

Merging #88 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #88   +/-   ##
=======================================
  Coverage   90.29%   90.29%           
=======================================
  Files          14       14           
  Lines        1442     1442           
=======================================
  Hits         1302     1302           
  Misses        140      140

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9a0fa3...96bc7fd. Read the comment docs.

@jonhoo jonhoo merged commit e992250 into jonhoo:master Mar 12, 2019
@jonhoo jonhoo mentioned this pull request Mar 14, 2019
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