Improve performance of Per Process data collection #71
Conversation
src/data/processes.rs
Outdated
| let user_time = values.nth(PROC_PID_STAT_USERSPACE_TIME_POS).unwrap().parse::<u64>()?; | ||
| let kernel_time = values.nth(0).unwrap().parse::<u64>()?; |
There was a problem hiding this comment.
Just because one of the magic values is 0 doesn't mean it should be left out of the constantification!
There was a problem hiding this comment.
I feel bad that the 0 got left out. I'm sorry!
src/data/processes.rs
Outdated
| let user_time = values.nth(PROC_PID_STAT_USERSPACE_TIME_POS).unwrap().parse::<u64>()?; | ||
| let kernel_time = values.nth(0).unwrap().parse::<u64>()?; |
There was a problem hiding this comment.
Is that right? The first column after the process executable is the state, not kernel time. System time looks to be after the user time.
There was a problem hiding this comment.
Calling nth() on an iterator consumes all the values before it. Since I called nth(11), the 12th value essentially is at index 0 after the call. See: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.nth. When I was testing this, nth(11) and nth(12) caused me to get the 11th value and the 23rd value -_- . Who does this?
There was a problem hiding this comment.
I didn't realize this was an iterator.
I don't think operating on it as such is a good idea, just collect it into an array (or split it straight to an array if there's an API for it). Much less fragile to refactorings later.
There was a problem hiding this comment.
Okay. There is a way to collect() to a Vec. I've used that.
| }; | ||
| processes.entries.push(sample); | ||
|
|
||
| let open_parenthesis = line.find('('); |
There was a problem hiding this comment.
When you've got parsing code like this, it's helpful to all parties if you add a comment with an example line, such as
34104 (cat) R 33725 34104 33725 34817 34104 4194304 83 0 0 0 0 0 0 0 20 0 1 0 61630149 115126272 99 18446744073709551615 4194304 4238576 281474576906576 0 0 0 0 0 0 0 0 0 17 8 0 0 0 0 0 4324336 4326088 233324544 281474576909547 281474576909567 281474576909567 281474576912363 0
There was a problem hiding this comment.
Gah. Yeah. That makes things clearer. Apologies.
There was a problem hiding this comment.
Still appears to be missing
janaknat
force-pushed
the
proc_performance
branch
from
04a05d3 to
f2e740c
Compare
Change the per process data collection implementation. Instead of using the procfs crate, manually read the
/procentries, loop through each/proc/[pid]and read the/proc/[pid]/statfile and store it. Without the parsing, the CPU utilization in a 64 vcpu system with a kernel build goes from ~(90-110ms) to about ~(50-60ms).By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.