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 scaled column for Allocated Memory #722

Closed
adamsitnik opened this issue Apr 18, 2018 · 7 comments · Fixed by #1859
Closed

Add scaled column for Allocated Memory #722

adamsitnik opened this issue Apr 18, 2018 · 7 comments · Fixed by #1859

Comments

@adamsitnik
Copy link
Member

Suggested by @stephentoub

There's also no equivalent for the memory columns that I could find, which makes things inconsistent in the results, since some of these benchmarks are more about memory than they are about throughput.

Side note: The column should be visible when one of the benchmarks is marked as baseline

@gsomix
Copy link
Contributor

gsomix commented Oct 8, 2018

I'm on it.

@adamsitnik
Copy link
Member Author

@gsomix could you wait until I merge #878 ? I should merge it in 24h. It contains some things that would break your PR for sure (it's touching the columns for diagnosers)

@gsomix
Copy link
Contributor

gsomix commented Oct 8, 2018

@adamsitnik OK, I'm just reading code for now. :)

@adamsitnik
Copy link
Member Author

@gsomix I have merged the #878

@gsomix
Copy link
Contributor

gsomix commented Oct 14, 2018

@adamsitnik I'm looking for some guidance for this issue. There are classes for baselines (e.g. BaselineCustomColumn) that tightly coupled with Statistics. We don't have any statistics for memory usage, only raw numbers of allocated bytes or gc operations. Should I somehow reuse and refactor exist baseline columns, or just to create new one (like MetricScaledColumn)?

@adamsitnik
Copy link
Member Author

@gsomix I think that you could detect if any of the Benchmarks have a basline, if so MemoryDiagnoser could return one more metric - scaled allocated bytes

@Kavignon
Copy link

I’ll take this if that’s ok!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants