When computing the product name, don't just pick the first item in the command array, because it might be an environment variable.

This fixes what's displayed in the top left corner in the profiler UI when
doing 'samply record MYENV=val my-command' .

Author: mstange

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<ServerProps>,
 ) -> Result<ExitStatus, ()> {
     // 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.

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<ServerProps>,
 ) -> Result<ExitStatus, MachError> {
-    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 || {

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,

samply/src/shared/recording_props.rs

@@ -1,16 +1,28 @@
-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<Duration>,
     pub interval: Duration,
     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.
     pub reuse_threads: bool,
     /// 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<OsString>,
+    pub iteration_count: u32,
+}