Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename JitdumpOrMarkerPath to ProcessSpecificPath, add aux_child to ExistingProcessRunner #355

Merged
merged 4 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions samply/src/mac/process_launcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub use super::mach_ipc::{mach_port_t, MachError, OsIpcSender};
use super::mach_ipc::{mach_task_self, BlockingMode, OsIpcMultiShotServer, MACH_PORT_NULL};

pub trait RootTaskRunner {
fn run_root_task(&self) -> Result<ExitStatus, MachError>;
fn run_root_task(&mut self) -> Result<ExitStatus, MachError>;
}

pub struct TaskLauncher {
Expand All @@ -30,7 +30,7 @@ pub struct TaskLauncher {
}

impl RootTaskRunner for TaskLauncher {
fn run_root_task(&self) -> Result<ExitStatus, MachError> {
fn run_root_task(&mut self) -> Result<ExitStatus, MachError> {
// Ignore Ctrl+C while the subcommand is running. The signal still reaches the process
// under observation while we continue to record it. (ctrl+c will send the SIGINT signal
// to all processes in the foreground process group).
Expand Down Expand Up @@ -217,6 +217,14 @@ impl TaskAccepter {
let path = &marker_file_info[5..][..len];
ReceivedStuff::MarkerFilePath(pid, OsStr::from_bytes(path).into())
}
(b"NetTrac", dotnet_trace_file_info) => {
let pid_bytes = &dotnet_trace_file_info[0..4];
let pid =
u32::from_le_bytes([pid_bytes[0], pid_bytes[1], pid_bytes[2], pid_bytes[3]]);
let len = dotnet_trace_file_info[4] as usize;
let path = &dotnet_trace_file_info[5..][..len];
ReceivedStuff::DotnetTracePath(pid, OsStr::from_bytes(path).into())
}
(other, _) => {
panic!("Unexpected message: {:?}", other);
}
Expand All @@ -229,6 +237,7 @@ pub enum ReceivedStuff {
AcceptedTask(AcceptedTask),
JitdumpPath(u32, PathBuf),
MarkerFilePath(u32, PathBuf),
DotnetTracePath(u32, PathBuf),
}

pub struct AcceptedTask {
Expand Down Expand Up @@ -257,10 +266,11 @@ impl AcceptedTask {

pub struct ExistingProcessRunner {
pid: u32,
aux_child: Option<Child>,
}

impl RootTaskRunner for ExistingProcessRunner {
fn run_root_task(&self) -> Result<ExitStatus, MachError> {
fn run_root_task(&mut self) -> Result<ExitStatus, MachError> {
let ctrl_c_receiver = CtrlC::observe_oneshot();

eprintln!("Profiling {}, press Ctrl-C to stop...", self.pid);
Expand All @@ -269,6 +279,16 @@ impl RootTaskRunner for ExistingProcessRunner {
.blocking_recv()
.expect("Ctrl+C receiver failed");

if let Some(aux_child) = self.aux_child.as_mut() {
let aux_pid = aux_child.id();
unsafe {
libc::kill(aux_pid as i32, libc::SIGINT);
}
aux_child
.wait()
.expect("Failed to wait on aux child process");
}

eprintln!("Done.");

Ok(ExitStatus::default())
Expand Down Expand Up @@ -309,6 +329,23 @@ impl ExistingProcessRunner {

// TODO: find all its children

ExistingProcessRunner { pid }
ExistingProcessRunner {
pid,
aux_child: None,
}
}

#[allow(unused)]
pub fn new_with_aux_child(
pid: u32,
task_accepter: &mut TaskAccepter,
aux_child: Child,
) -> ExistingProcessRunner {
let runner = Self::new(pid, task_accepter);

ExistingProcessRunner {
aux_child: Some(aux_child),
..runner
}
}
}
60 changes: 43 additions & 17 deletions samply/src/mac/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::error::SamplingError;
use super::process_launcher::{
ExistingProcessRunner, MachError, ReceivedStuff, RootTaskRunner, TaskAccepter, TaskLauncher,
};
use super::sampler::{JitdumpOrMarkerPath, Sampler, TaskInit, TaskInitOrShutdown};
use super::sampler::{ProcessSpecificPath, Sampler, TaskInit, TaskInitOrShutdown};
use super::time::get_monotonic_timestamp;
use crate::server::{start_server_main, ServerProps};
use crate::shared::recording_props::{
Expand All @@ -31,7 +31,7 @@ pub fn start_recording(

let mut task_accepter = TaskAccepter::new()?;

let root_task_runner: Box<dyn RootTaskRunner> = match recording_mode {
let mut root_task_runner: Box<dyn RootTaskRunner> = match recording_mode {
RecordingMode::All => {
eprintln!("Error: Profiling all processes is not supported on macOS.");
eprintln!("You can only profile processes which you launch via samply, or attach to via --pid.");
Expand All @@ -46,24 +46,34 @@ pub fn start_recording(
iteration_count,
} = process_launch_props;

if profile_creation_props.coreclr.any_enabled() {
// We need to set DOTNET_PerfMapEnabled=2 in the environment if it's not already set.
let task_launcher = if profile_creation_props.coreclr.any_enabled() {
// We need to set DOTNET_PerfMapEnabled=3 in the environment if it's not already set.
// If we set it, we'll also set unlink_aux_files=true to avoid leaving files
// behind in the temp directory. But if it's set manually, assume the user
// knows what they're doing and will specify the arg as needed.
if !env_vars.iter().any(|p| p.0 == "DOTNET_PerfMapEnabled") {
env_vars.push(("DOTNET_PerfMapEnabled".into(), "2".into()));
env_vars.push(("DOTNET_PerfMapEnabled".into(), "3".into()));
profile_creation_props.unlink_aux_files = true;
}
}

let task_launcher = TaskLauncher::new(
&command_name,
&args,
iteration_count,
&env_vars,
task_accepter.extra_env_vars(),
)?;
// To be filled in with new launching code in future PR

TaskLauncher::new(
&command_name,
&args,
iteration_count,
&env_vars,
task_accepter.extra_env_vars(),
)?
} else {
TaskLauncher::new(
&command_name,
&args,
iteration_count,
&env_vars,
task_accepter.extra_env_vars(),
)?
};

Box::new(task_launcher)
}
Expand Down Expand Up @@ -113,7 +123,7 @@ pub fn start_recording(
match path_senders_per_pid.entry(pid) {
Entry::Occupied(mut entry) => {
let send_result =
entry.get_mut().send(JitdumpOrMarkerPath::JitdumpPath(path));
entry.get_mut().send(ProcessSpecificPath::Jitdump(path));
if send_result.is_err() {
// The task is probably already dead. The path arrived too late.
entry.remove();
Expand All @@ -129,9 +139,25 @@ pub fn start_recording(
Ok(ReceivedStuff::MarkerFilePath(pid, path)) => {
match path_senders_per_pid.entry(pid) {
Entry::Occupied(mut entry) => {
let send_result = entry
.get_mut()
.send(JitdumpOrMarkerPath::MarkerFilePath(path));
let send_result =
entry.get_mut().send(ProcessSpecificPath::MarkerFile(path));
if send_result.is_err() {
// The task is probably already dead. The path arrived too late.
entry.remove();
}
}
Entry::Vacant(_entry) => {
eprintln!(
"Received a marker file path for pid {pid} which I don't have a task for."
);
}
}
}
Ok(ReceivedStuff::DotnetTracePath(pid, path)) => {
match path_senders_per_pid.entry(pid) {
Entry::Occupied(mut entry) => {
let send_result =
entry.get_mut().send(ProcessSpecificPath::DotnetTrace(path));
if send_result.is_err() {
// The task is probably already dead. The path arrived too late.
entry.remove();
Expand Down
10 changes: 6 additions & 4 deletions samply/src/mac/sampler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ use crate::shared::recycling::ProcessRecycler;
use crate::shared::timestamp_converter::TimestampConverter;
use crate::shared::unresolved_samples::UnresolvedStacks;

pub enum JitdumpOrMarkerPath {
JitdumpPath(PathBuf),
MarkerFilePath(PathBuf),
pub enum ProcessSpecificPath {
Jitdump(PathBuf),
MarkerFile(PathBuf),
#[allow(unused)]
DotnetTrace(PathBuf),
}

#[derive(Debug, Clone)]
Expand All @@ -31,7 +33,7 @@ pub struct TaskInit {
pub start_time_mono: u64,
pub task: mach_port_t,
pub pid: u32,
pub path_receiver: Receiver<JitdumpOrMarkerPath>,
pub path_receiver: Receiver<ProcessSpecificPath>,
}

pub struct Sampler {
Expand Down
15 changes: 9 additions & 6 deletions samply/src/mac/task_profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use super::proc_maps::{
proc_cmdline, DyldInfo, DyldInfoManager, Modification, ModuleSvmaInfo, StackwalkerRef,
VmSubData,
};
use super::sampler::{JitdumpOrMarkerPath, TaskInit};
use super::sampler::{ProcessSpecificPath, TaskInit};
use super::thread_profiler::{get_thread_id, get_thread_name, ThreadProfiler};
use crate::shared::jit_category_manager::JitCategoryManager;
use crate::shared::jit_function_recycler::JitFunctionRecycler;
Expand Down Expand Up @@ -108,7 +108,7 @@ pub struct TaskProfiler {
main_thread_label_frame: FrameInfo,
ignored_errors: Vec<SamplingError>,
unwinder: UnwinderNative<UnwindSectionBytes, MayAllocateDuringUnwind>,
path_receiver: Receiver<JitdumpOrMarkerPath>,
path_receiver: Receiver<ProcessSpecificPath>,
jitdump_manager: JitDumpManager,
marker_file_paths: Vec<(ThreadHandle, PathBuf)>,
unresolved_samples: UnresolvedSamples,
Expand Down Expand Up @@ -585,9 +585,9 @@ impl TaskProfiler {
}

pub fn check_received_paths(&mut self) {
while let Ok(jitdump_or_marker_file_path) = self.path_receiver.try_recv() {
match jitdump_or_marker_file_path {
JitdumpOrMarkerPath::JitdumpPath(jitdump_path) => {
while let Ok(process_specific_path) = self.path_receiver.try_recv() {
match process_specific_path {
ProcessSpecificPath::Jitdump(jitdump_path) => {
// TODO: Detect which thread the jitdump file is opened on, and use that thread's
// thread handle so that the JitFunctionAdd markers are put on that thread in the profile.
self.jitdump_manager.add_jitdump_path(
Expand All @@ -596,7 +596,7 @@ impl TaskProfiler {
Vec::new(),
);
}
JitdumpOrMarkerPath::MarkerFilePath(marker_file_path) => {
ProcessSpecificPath::MarkerFile(marker_file_path) => {
// count the number of - characters in marker_file_path
let marker_info = marker_file::parse_marker_file_path(&marker_file_path);
let thread_handle = if marker_info.tid.is_some() {
Expand All @@ -611,6 +611,9 @@ impl TaskProfiler {
self.marker_file_paths
.push((thread_handle, marker_file_path));
}
ProcessSpecificPath::DotnetTrace(_) => {
// nothing, for now
}
}
}
}
Expand Down