Skip to content

Commit

Permalink
Add a --cswitch-markers flag.
Browse files Browse the repository at this point in the history
This allows using --per-cpu-threads without bloating the profile size too much.

Sometimes you just want to see samples across all threads and don't care
when exactly each thread started and stopped running on a CPU.
  • Loading branch information
mstange committed Dec 7, 2024
1 parent e28a235 commit 9165d44
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 40 deletions.
42 changes: 25 additions & 17 deletions samply/src/linux_shared/converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ where

/// Whether to emit JitFunctionAdd markers.
should_emit_jit_markers: bool,

/// Whether to emit context switch markers.
should_emit_cswitch_markers: bool,
}

const DEFAULT_OFF_CPU_SAMPLING_INTERVAL_NS: u64 = 1_000_000; // 1ms
Expand Down Expand Up @@ -272,6 +275,7 @@ where
cpus,
call_chain_return_addresses_are_preadjusted,
should_emit_jit_markers: profile_creation_props.should_emit_jit_markers,
should_emit_cswitch_markers: profile_creation_props.should_emit_cswitch_markers,
}
}

Expand Down Expand Up @@ -962,14 +966,16 @@ where
Some(idle_frame_label),
);
}
cpu.notify_switch_in(
tid,
thread.thread_label(),
timestamp,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
&mut self.profile,
);
if self.should_emit_cswitch_markers {
cpu.notify_switch_in_for_marker(
tid,
thread.thread_label(),
timestamp,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
&mut self.profile,
);
}
}
}
ContextSwitchRecord::Out { preempted, .. } => {
Expand All @@ -980,15 +986,17 @@ where
let cpu = cpus.get_mut(cpu_index as usize, &mut self.profile);
self.context_switch_handler
.handle_switch_out(timestamp, &mut cpu.context_switch_data);
cpu.notify_switch_out(
tid,
timestamp,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
thread.profile_thread,
preempted == TaskWasPreempted::Yes,
&mut self.profile,
);
if self.should_emit_cswitch_markers {
cpu.notify_switch_out_for_marker(
tid,
timestamp,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
thread.profile_thread,
preempted == TaskWasPreempted::Yes,
&mut self.profile,
);
}
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions samply/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,10 @@ pub struct ProfileCreationArgs {
#[arg(long)]
jit_markers: bool,

/// Emit context switch markers.
#[arg(long)]
cswitch_markers: bool,

/// Include up to <INCLUDE_ARGS> command line arguments in the process name.
/// This can help differentiate processes if the same executable is used
/// for different types of programs. And in --reuse-threads mode it
Expand Down Expand Up @@ -546,6 +550,7 @@ impl ImportArgs {
override_arch: self.override_arch.clone(),
unstable_presymbolicate: self.profile_creation_args.unstable_presymbolicate,
should_emit_jit_markers: self.profile_creation_args.jit_markers,
should_emit_cswitch_markers: self.profile_creation_args.cswitch_markers,
coreclr: to_coreclr_profile_props(&self.coreclr),
#[cfg(target_os = "windows")]
unknown_event_markers: self.profile_creation_args.unknown_event_markers,
Expand Down Expand Up @@ -670,6 +675,7 @@ impl RecordArgs {
override_arch: None,
unstable_presymbolicate: self.profile_creation_args.unstable_presymbolicate,
should_emit_jit_markers: self.profile_creation_args.jit_markers,
should_emit_cswitch_markers: self.profile_creation_args.cswitch_markers,
coreclr: to_coreclr_profile_props(&self.coreclr),
#[cfg(target_os = "windows")]
unknown_event_markers: self.profile_creation_args.unknown_event_markers,
Expand Down
6 changes: 3 additions & 3 deletions samply/src/shared/per_cpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub struct Cpu {
pub name: StringHandle,
pub thread_handle: ThreadHandle,
pub context_switch_data: ThreadContextSwitchData,
pub current_tid: Option<(i32, StringHandle, u64)>,
current_tid: Option<(i32, StringHandle, u64)>,
}

impl Cpu {
Expand All @@ -33,7 +33,7 @@ impl Cpu {
}

#[allow(clippy::too_many_arguments)]
pub fn notify_switch_in(
pub fn notify_switch_in_for_marker(
&mut self,
tid: i32,
thread_name: StringHandle,
Expand All @@ -60,7 +60,7 @@ impl Cpu {
}

#[allow(clippy::too_many_arguments)]
pub fn notify_switch_out(
pub fn notify_switch_out_for_marker(
&mut self,
tid: i32, // tid that is being switched away from
timestamp: u64,
Expand Down
3 changes: 3 additions & 0 deletions samply/src/shared/recording_props.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ pub struct ProfileCreationProps {
/// Whether to emit "JitFunctionAdd" markers.
#[allow(dead_code)]
pub should_emit_jit_markers: bool,
/// Whether to emit context switch markers.
#[allow(dead_code)]
pub should_emit_cswitch_markers: bool,
}

impl ProfileCreationProps {
Expand Down
44 changes: 24 additions & 20 deletions samply/src/windows/profile_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1630,18 +1630,20 @@ impl ProfileContext {
self.context_switch_handler
.handle_switch_out(timestamp_raw, &mut cpu.context_switch_data);

// TODO: Find out if this actually is the right way to check whether a thread
// has been pre-empted.
let preempted = wait_reason == 0 || wait_reason == 32; // "Executive" | "WrPreempted"
cpu.notify_switch_out(
old_tid as i32,
timestamp_raw,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
old_thread.handle,
preempted,
&mut self.profile,
);
if self.profile_creation_props.should_emit_cswitch_markers {
// TODO: Find out if this actually is the right way to check whether a thread
// has been pre-empted.
let preempted = wait_reason == 0 || wait_reason == 32; // "Executive" | "WrPreempted"
cpu.notify_switch_out_for_marker(
old_tid as i32,
timestamp_raw,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
old_thread.handle,
preempted,
&mut self.profile,
);
}
}
}

Expand Down Expand Up @@ -1705,14 +1707,16 @@ impl ProfileContext {
0,
);
}
cpu.notify_switch_in(
new_tid as i32,
new_thread.thread_label(),
timestamp_raw,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
&mut self.profile,
);
if self.profile_creation_props.should_emit_cswitch_markers {
cpu.notify_switch_in_for_marker(
new_tid as i32,
new_thread.thread_label(),
timestamp_raw,
&self.timestamp_converter,
&[cpu.thread_handle, combined_thread],
&mut self.profile,
);
}
}
}
}
Expand Down

0 comments on commit 9165d44

Please sign in to comment.