Wait for 'perf record' to finish #160
Conversation
src/data/flamegraphs.rs
Outdated
| let mut child = PERF_CHILD.lock().unwrap(); | ||
| match child.as_ref() { | ||
| None => return Ok(()), | ||
| Some(_) => {} | ||
| } | ||
|
|
||
| trace!("Waiting for perf profile collection to complete..."); | ||
| match child.as_mut().unwrap().wait() { | ||
| Err(e) => { | ||
| error!("'perf' did not exit successfully: {}", e); | ||
| return Ok(()); | ||
| } | ||
| Ok(_) => trace!("'perf record' executed successfully."), | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you try to do this more generically, like have all data collectors have a synchronous shutdown/stop, even if for most of them it's a no-op, so that this kind of code can be kept hidden/separated from things that shouldn't know about it?
There was a problem hiding this comment.
Ok. I've added a finish_data_collection step.
| lazy_static! { | ||
| pub static ref PERF_CHILD: Mutex<Option<Child>> = Mutex::new(None); | ||
| } |
There was a problem hiding this comment.
Can't you put this state into the PerfProfileRaw struct?
There was a problem hiding this comment.
The Child type is not Serializable. We have that constraint since the PerfProfileRaw struct is part of the Data enum.
There was a problem hiding this comment.
That really needs to be fixed. The same type that's being serialized as output data shouldn't be the one doing the data collection, it causes issues like this.
The time taken by perf record after the collection period depends on the time of collection and the system load. On heavily loaded systems, profiling can take longer than the time it takes to go from perf collection to perf inject in the post collection step. To prevent a race condition, add a wait before the perf inject step.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.