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

allow scopes to flush between each sample. #153

Merged
merged 1 commit into from
Jun 22, 2023
Merged

Conversation

tgolsson
Copy link
Member

Benchmarks run fine to the end, but then dies to memory kill at exit. The scope at the top seems to prevent flushing, so all data is kept in memory until we exit. By only delaying flushing until after each benchmark we can still hide it from measurement (assuming bench_function isn't measured) but prevent memory growth between benches. Adding the new_frame just for good measure as it allows dropping the data progressively too.

@tgolsson
Copy link
Member Author

@andreiltd Did you have anything that you were investigating beyond this that you'd want to combine this with? Did you find a good explanation for why this improved perf beyond me guessing? :D

@andreiltd
Copy link
Member

LGTM, although I'm not sure if collecting frames affects the profiled function in any way or if this occurs entirely outside the measured scope?

@tgolsson
Copy link
Member Author

From the criterion docs, my understand is that only b.iter() is measured. So it should be unchanged. Which makes your detected diff a even weirder.

@TimonPost TimonPost removed the request for review from emilk June 22, 2023 10:47
@mergify mergify bot merged commit 6141817 into main Jun 22, 2023
@mergify mergify bot deleted the ts/flush-samples branch June 22, 2023 10:47
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