diff --git a/samply/src/linux/profiler.rs b/samply/src/linux/profiler.rs index 0b118427..0978620f 100644 --- a/samply/src/linux/profiler.rs +++ b/samply/src/linux/profiler.rs @@ -27,7 +27,7 @@ use crate::linux_shared::{ ConvertRegs, Converter, EventInterpretation, MmapRangeOrVec, OffCpuIndicator, }; use crate::server::{start_server_main, ServerProps}; -use crate::shared::recording_props::{ProfileCreationProps, RecordingProps}; +use crate::shared::recording_props::{ProcessLaunchProps, ProfileCreationProps, RecordingProps}; #[cfg(target_arch = "x86_64")] pub type ConvertRegsNative = crate::linux_shared::ConvertRegsX86_64; @@ -37,16 +37,20 @@ pub type ConvertRegsNative = crate::linux_shared::ConvertRegsAarch64; #[allow(clippy::too_many_arguments)] pub fn start_recording( - command_name: OsString, - command_args: &[OsString], - env_vars: &[(OsString, OsString)], - iteration_count: u32, + process_launch_props: ProcessLaunchProps, recording_props: RecordingProps, profile_creation_props: ProfileCreationProps, server_props: Option, ) -> Result { // We want to profile a child process which we are about to launch. + let ProcessLaunchProps { + env_vars, + command_name, + args, + iteration_count, + } = process_launch_props; + // Ignore SIGINT in our process while the child process is running. The // signal will still reach the child process, because Ctrl+C sends the // SIGINT signal to all processes in the foreground process group. @@ -61,7 +65,7 @@ pub fn start_recording( // Start a new process for the launched command and get its pid. // The command will not start running until we tell it to. let process = - SuspendedLaunchedProcess::launch_in_suspended_state(&command_name, command_args, env_vars) + SuspendedLaunchedProcess::launch_in_suspended_state(&command_name, &args, &env_vars) .unwrap(); let pid = process.pid(); @@ -158,12 +162,9 @@ pub fn start_recording( break; } eprintln!("Running iteration {i} of {iteration_count}..."); - let process = SuspendedLaunchedProcess::launch_in_suspended_state( - &command_name, - command_args, - env_vars, - ) - .unwrap(); + let process = + SuspendedLaunchedProcess::launch_in_suspended_state(&command_name, &args, &env_vars) + .unwrap(); let pid = process.pid(); // Tell the sampler to start profiling another pid, and wait for it to signal us to go ahead. diff --git a/samply/src/mac/profiler.rs b/samply/src/mac/profiler.rs index 083a5a59..bfc2913d 100644 --- a/samply/src/mac/profiler.rs +++ b/samply/src/mac/profiler.rs @@ -3,7 +3,6 @@ use serde_json::to_writer; use std::collections::hash_map::Entry; use std::collections::HashMap; -use std::ffi::OsString; use std::fs::File; use std::io::BufWriter; use std::process::ExitStatus; @@ -17,7 +16,7 @@ use super::process_launcher::{MachError, ReceivedStuff, TaskAccepter}; use super::sampler::{JitdumpOrMarkerPath, Sampler, TaskInit}; use super::time::get_monotonic_timestamp; use crate::server::{start_server_main, ServerProps}; -use crate::shared::recording_props::{ProfileCreationProps, RecordingProps}; +use crate::shared::recording_props::{ProcessLaunchProps, ProfileCreationProps, RecordingProps}; pub fn start_profiling_pid( _pid: u32, @@ -31,17 +30,22 @@ pub fn start_profiling_pid( } pub fn start_recording( - command_name: OsString, - command_args: &[OsString], - env_vars: &[(OsString, OsString)], - iteration_count: u32, + process_launch_props: ProcessLaunchProps, recording_props: RecordingProps, profile_creation_props: ProfileCreationProps, server_props: Option, ) -> Result { - let (task_sender, task_receiver) = unbounded(); + let ProcessLaunchProps { + env_vars, + command_name, + args, + iteration_count, + } = process_launch_props; let command_name_copy = command_name.to_string_lossy().to_string(); let output_file = recording_props.output_file.clone(); + + let (task_sender, task_receiver) = unbounded(); + let sampler_thread = thread::spawn(move || { let sampler = Sampler::new( command_name_copy, @@ -63,8 +67,7 @@ pub fn start_recording( ) .expect("cannot register signal handler"); - let (mut task_accepter, task_launcher) = - TaskAccepter::new(&command_name, command_args, env_vars)?; + let (mut task_accepter, task_launcher) = TaskAccepter::new(&command_name, &args, &env_vars)?; let (accepter_sender, accepter_receiver) = unbounded(); let accepter_thread = thread::spawn(move || { diff --git a/samply/src/main.rs b/samply/src/main.rs index 442489b7..7acbc4b0 100644 --- a/samply/src/main.rs +++ b/samply/src/main.rs @@ -12,9 +12,9 @@ mod shared; use clap::{Args, Parser, Subcommand}; use profile_json_preparse::parse_libinfo_map_from_profile_file; -use shared::recording_props::{ProfileCreationProps, RecordingProps}; +use shared::recording_props::{ProcessLaunchProps, ProfileCreationProps, RecordingProps}; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsStr; use std::fs::File; use std::io::{BufReader, BufWriter}; use std::path::{Path, PathBuf}; @@ -237,6 +237,7 @@ fn main() { #[cfg(any(target_os = "android", target_os = "macos", target_os = "linux"))] Action::Record(record_args) => { + let process_launch_props = record_args.process_launch_props(); let recording_props = record_args.recording_props(); let profile_creation_props = record_args.profile_creation_props(); let server_props = record_args.server_props(); @@ -249,12 +250,8 @@ fn main() { server_props, ); } else { - let (env_vars, command_name, args) = parse_command(&record_args.command); let exit_status = match profiler::start_recording( - command_name, - args, - &env_vars, - record_args.iteration_count, + process_launch_props, recording_props, profile_creation_props, server_props, @@ -330,13 +327,45 @@ impl RecordArgs { } } + pub fn process_launch_props(&self) -> ProcessLaunchProps { + let command = &self.command; + let iteration_count = self.iteration_count; + assert!( + !command.is_empty(), + "CLI parsing should have ensured that we have at least one command name" + ); + + let mut env_vars = Vec::new(); + let mut i = 0; + while let Some((var_name, var_val)) = command.get(i).and_then(|s| split_at_first_equals(s)) + { + env_vars.push((var_name.to_owned(), var_val.to_owned())); + i += 1; + } + if i == command.len() { + eprintln!("Error: No command name found. Every item looks like an environment variable (contains '='): {command:?}"); + std::process::exit(1); + } + let command_name = command[i].clone(); + let args = command[(i + 1)..].to_owned(); + ProcessLaunchProps { + env_vars, + command_name, + args, + iteration_count, + } + } + #[allow(unused)] pub fn profile_creation_props(&self) -> ProfileCreationProps { - let profile_name = match (self.profile_creation_args.profile_name.clone(), self.pid, self.command.first()) { - (Some(profile_name), _, _) => profile_name, - (None, Some(pid), _) => format!("PID {pid}"), - (None, None, Some(command)) => command.to_string_lossy().to_string(), - (None, None, None) => panic!("Either pid or command is guaranteed to be present (clap should have done the validation)"), + let profile_name = match (self.profile_creation_args.profile_name.clone(), self.pid) { + (Some(profile_name), _) => profile_name, + (None, Some(pid)) => format!("PID {pid}"), + _ => self + .process_launch_props() + .command_name + .to_string_lossy() + .to_string(), }; ProfileCreationProps { profile_name, @@ -384,28 +413,6 @@ fn split_at_first_equals(s: &OsStr) -> Option<(&OsStr, &OsStr)> { Some((name, val)) } -#[allow(unused)] -fn parse_command(command: &[OsString]) -> (Vec<(OsString, OsString)>, OsString, &[OsString]) { - assert!( - !command.is_empty(), - "CLI parsing should have ensured that we have at least one command name" - ); - - let mut env_vars = Vec::new(); - let mut i = 0; - while let Some((var_name, var_val)) = command.get(i).and_then(|s| split_at_first_equals(s)) { - env_vars.push((var_name.to_owned(), var_val.to_owned())); - i += 1; - } - if i == command.len() { - eprintln!("Error: No command name found. Every item looks like an environment variable (contains '='): {command:?}"); - std::process::exit(1); - } - let command_name = command[i].clone(); - let args = &command[(i + 1)..]; - (env_vars, command_name, args) -} - fn convert_file_to_profile( filename: &Path, input_file: &File, diff --git a/samply/src/shared/recording_props.rs b/samply/src/shared/recording_props.rs index a6d392ed..f5d84bf6 100644 --- a/samply/src/shared/recording_props.rs +++ b/samply/src/shared/recording_props.rs @@ -1,5 +1,7 @@ -use std::{path::PathBuf, time::Duration}; +use std::{ffi::OsString, path::PathBuf, time::Duration}; +/// Properties which are meaningful both for recording a fresh process +/// as well as for recording an existing process. pub struct RecordingProps { pub output_file: PathBuf, pub time_limit: Option, @@ -7,6 +9,8 @@ pub struct RecordingProps { pub main_thread_only: bool, } +/// Properties which are meaningful both for recording a profile and +/// for converting a perf.data file to a profile. pub struct ProfileCreationProps { pub profile_name: String, /// Merge non-overlapping threads of the same name. @@ -14,3 +18,11 @@ pub struct ProfileCreationProps { /// Fold repeated frames at the base of the stack. pub fold_recursive_prefix: bool, } + +/// Properties which are meaningful for launching and recording a fresh process. +pub struct ProcessLaunchProps { + pub env_vars: Vec<(OsString, OsString)>, + pub command_name: OsString, + pub args: Vec, + pub iteration_count: u32, +} \ No newline at end of file