-
Notifications
You must be signed in to change notification settings - Fork 32
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 ability to clear statistics #118
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Zach <zachmgoldstein@gmail.com>
480574c
to
073e1c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the questions inline, it would also be great to have tests for this new functionality.
|
||
output->minor_pagefaults = std::accumulate( | ||
this->sample_buffer.minor_pagefaults.begin(), | ||
this->sample_buffer.minor_pagefaults.begin() + this->cleared_iteration + 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding 1 here? Won't that always skip a sample somewhere along the way?
// Reset the properties of the current results | ||
this->results.max_latency = this->sample_buffer.latency_samples[i]; | ||
this->results.min_latency = this->results.max_latency; | ||
this->results.mean_latency = this->results.max_latency; | ||
this->results.minor_pagefaults = this->sample_buffer.minor_pagefaults[i]; | ||
this->results.major_pagefaults = this->sample_buffer.major_pagefaults[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could be wrong in my understanding here, but this isn't really "clearing" the results. What it is doing is setting a marker at the current set of samples, and then we'll only consider samples from this point forward.
If my understanding is correct, then I think we should rename this function to reflect that usage more. Maybe "mark_statistics" or something like that?
See #117
Implements the functionality for clearing all statistics in the sample buffer
and the results struct by adding a API method that the user can call
while rttest is running.