Move profiling processing to the record step #206
Conversation
src/data/flamegraphs.rs
Outdated
| let out = Command::new("perf") | ||
| .args(["script", "-f", "-i", perf_jit_loc.to_str().unwrap()]) | ||
| .output(); | ||
| match out { | ||
| Err(e) => { | ||
| let out = format!("Did not process profiling data due to: {}", e); | ||
| error!("{}", out); | ||
| write_msg_to_svg(fg_out, out)?; | ||
| } | ||
| Ok(v) => { | ||
| write!(script_out, "{}", std::str::from_utf8(&v.stdout)?)?; |
There was a problem hiding this comment.
Can you try to get this so that 'perf' is writing the file directly, versus slurping a potentially huge output up into this process, only to then write it out to a file?
There was a problem hiding this comment.
Yeah. We can set the stdout and give it the file and that should work.
janaknat
force-pushed
the
pfl-in-record
branch
2 times, most recently
from
107c798
to
cbd1ed5
Compare
| let out = Command::new("perf") | ||
| .stdout(File::create(&script_loc)?) | ||
| .args(["script", "-f", "-i", perf_jit_loc.to_str().unwrap()]) |
There was a problem hiding this comment.
For the other invocations of perf you use it's -o option rather than stdout. I don't think there's a functional problem here, but it would be better to be consistent on how you're calling it.
There was a problem hiding this comment.
Most other perf sub-tools provide the -o option. perf script does not :(
src/data/flamegraphs.rs
Outdated
| let out_jit = Command::new("perf") | ||
| .args([ | ||
| "inject", | ||
| "-j", | ||
| "-i", | ||
| &file_pathbuf.to_str().unwrap(), | ||
| "-o", | ||
| perf_jit_loc.clone().to_str().unwrap(), | ||
| perf_jit_loc.to_str().unwrap(), | ||
| ]) | ||
| .status(); | ||
| .output(); | ||
|
|
||
| let fg_out = File::create(data_dir.join(format!("{}-flamegraph.svg", params.run_name)))?; | ||
|
|
||
| match out_jit { | ||
| Err(e) => { | ||
| if e.kind() == ErrorKind::NotFound { | ||
| error!("'perf' command not found."); | ||
| } else { | ||
| error!("Unknown error: {}", e); | ||
| let out = format!("Skip processing profiling data due to: {}", e); | ||
| error!("{}", out); | ||
| write_msg_to_svg(fg_out, out)?; |
There was a problem hiding this comment.
What are you expecting to get out of perf's output since you're telling it to write to a file? Errors would go to stderr anyway.
Can't this go back to only looking at status?
There was a problem hiding this comment.
Yeah. We should use .status(). I'll change it.
Currently, users are unable to generate reports, of an aperf run with --profile, on a system that is not the SUT. Move the processing step for perf_profile and flamegraph to the record stage.
Currently, users are unable to generate reports, of an aperf run with --profile, on a system that is not the SUT. Move the processing step for perf_profile and flamegraph to the record stage.
Testing:
Prior to this change, it was not possible to generate a comparison of profiling between runs in arm64 and x86.
arm64profile.tar.gz
x86profile.tar.gz
profilecmp.tar.gz
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.