Skip to content

Commit fbb5fab

Browse files
fix(hydro_deploy): handle -1 addresses from samply, fix _counter() rollover (#1814)
This fixes samply profiling on my "ancient" 2019 x86-64 macbook pro 15.3.2 (24D81) This pull request aims to fix the handling of –1 address values from samply by updating tracing filenames and refactoring related error and type handling. Key changes include: - Better error messages when `dtrace` or `samply` are not instaled. - Fix integer rollover in `_counter()` by using `u64` instead of inferred `i32`. - Refactor samply profile conversion for asynchronous frame lookup. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | hydro_lang/src/rewrites/analyze_counter.rs | Adds custom panic with measurement details if regex matching fails. (Used to diagnose `_counter()` `i32` rollover) | | hydro_deploy/core/src/localhost/samply.rs | Updates type for addresses/resources, refactors frame lookup to use asynchronous join_all, and adjusts string output for missing symbols. | | hydro_deploy/core/src/localhost/mod.rs | Improves error handling during command spawning with conditional context messages for when `samply` or `dtrace` executables are not found. | | hydro_deploy/core/src/localhost/launched_binary.rs | Uses serde_path_to_error for improved deserialization error context. | | dfir_lang/src/graph/ops/dest_sink.rs | Standardizes error messages by removing extraneous punctuation. | | dfir_lang/src/graph/ops/_counter.rs | Adds explicit type annotation for a cell initialization to prevent `i32` rollover. | </details>
1 parent 6d24901 commit fbb5fab

File tree

11 files changed

+73
-49
lines changed

11 files changed

+73
-49
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ flamegraph.svg
1515
/*.data.folded
1616
/*.perf.data
1717
/*.svg
18+
/*.profile

Cargo.lock

+11
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cleanup-perf.bash

-2
This file was deleted.

cleanup-tracing.bash

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#!/usr/bin/env bash
2+
rm *.data.folded *.perf.data *.svg *.profile || true

dfir_lang/src/graph/ops/_counter.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ pub const _COUNTER: OperatorConstraints = OperatorConstraints {
6868
let duration_ident = wc.make_ident("duration");
6969

7070
let write_prologue = quote_spanned! {op_span=>
71-
let #write_ident = ::std::rc::Rc::new(::std::cell::Cell::new(0));
71+
let #write_ident = ::std::rc::Rc::new(::std::cell::Cell::new(0_u64));
7272

7373
let #read_ident = ::std::rc::Rc::clone(&#write_ident);
7474
let #duration_ident = #duration_expr;

dfir_lang/src/graph/ops/dest_sink.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,13 @@ pub const DEST_SINK: OperatorConstraints = OperatorConstraints {
131131
while let Some(item) = recv.recv().await {
132132
sink.feed(item)
133133
.await
134-
.expect("Error processing async sink item.");
134+
.expect("Error processing async sink item");
135135
while let Ok(item) = recv.try_recv() {
136136
sink.feed(item)
137137
.await
138-
.expect("Error processing async sink item.");
138+
.expect("Error processing async sink item");
139139
}
140-
sink.flush().await.expect("Failed to flush sink.");
140+
sink.flush().await.expect("Failed to flush sink");
141141
}
142142
}
143143
#df_ident
@@ -148,7 +148,7 @@ pub const DEST_SINK: OperatorConstraints = OperatorConstraints {
148148
let write_iterator = quote_spanned! {op_span=>
149149
let #ident = #root::pusherator::for_each::ForEach::new(|item| {
150150
if let Err(err) = #send_ident.send(item) {
151-
panic!("Failed to send async write item for processing.: {}", err);
151+
panic!("Failed to send async write item for processing: {}", err);
152152
}
153153
});
154154
};

hydro_deploy/core/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ nanoid = "0.4.0"
3333
nix = { version = "0.29.0", features = ["signal"] }
3434
serde = { version = "1.0.197", features = ["derive"] }
3535
serde_json = "1.0.115"
36+
serde_path_to_error = "0.1.0"
3637
shell-escape = "0.1.0"
3738
tempfile = "3.0.0"
3839
tokio = { version = "1.29.0", features = ["full"] }

hydro_deploy/core/src/localhost/launched_binary.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,10 @@ impl LaunchedBinary for LaunchedLocalhostBinary {
188188
}
189189

190190
let fold_data = if cfg!(target_os = "macos") {
191-
let loaded = serde_json::from_reader::<_, FxProfile>(std::fs::File::open(
192-
tracing_data.outfile.path(),
193-
)?)?;
191+
let deserializer = &mut serde_json::Deserializer::from_reader(
192+
std::fs::File::open(tracing_data.outfile.path())?,
193+
);
194+
let loaded = serde_path_to_error::deserialize::<_, FxProfile>(deserializer)?;
194195

195196
samply_to_folded(loaded).await.into()
196197
} else if cfg!(target_family = "windows") {

hydro_deploy/core/src/localhost/mod.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::collections::HashMap;
22
use std::net::SocketAddr;
33
use std::sync::Arc;
44

5-
use anyhow::{Context, Result, bail};
5+
use anyhow::{Result, bail};
66
use async_process::{Command, Stdio};
77
use async_trait::async_trait;
88
use hydro_deploy_integration::ServerBindConfig;
@@ -214,9 +214,14 @@ impl LaunchedHost for LaunchedLocalhost {
214214

215215
ProgressTracker::println(format!("[{}] running command: `{:?}`", id, command));
216216

217-
let child = command
218-
.spawn()
219-
.with_context(|| format!("Failed to execute command: {:?}", command))?;
217+
let child = command.spawn().map_err(|e| {
218+
let msg = if maybe_perf_outfile.is_some() && std::io::ErrorKind::NotFound == e.kind() {
219+
"Tracing executable not found, ensure it is installed"
220+
} else {
221+
"Failed to execute command"
222+
};
223+
anyhow::Error::new(e).context(format!("{}: {:?}", msg, command))
224+
})?;
220225

221226
Ok(Box::new(LaunchedLocalhostBinary::new(
222227
child,

hydro_deploy/core/src/localhost/samply.rs

+34-34
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ use std::collections::BTreeMap;
22
use std::path::PathBuf;
33
use std::str::FromStr;
44

5+
use futures::future::join_all;
56
use itertools::Itertools;
67
use serde::{Deserialize, Serialize};
8+
use serde_json::Number;
79
use wholesym::debugid::DebugId;
810
use wholesym::{LookupAddress, MultiArchDisambiguator, SymbolManager, SymbolManagerConfig};
911

@@ -47,13 +49,15 @@ pub struct StackTable {
4749

4850
#[derive(Serialize, Deserialize, Debug)]
4951
pub struct FrameTable {
50-
pub address: Vec<u64>,
52+
/// `Vec` of `u64` or `-1`.
53+
pub address: Vec<Number>,
5154
pub func: Vec<usize>,
5255
}
5356

5457
#[derive(Serialize, Deserialize, Debug)]
5558
pub struct FuncTable {
56-
pub resource: Vec<usize>,
59+
// `Vec` of `usize` or `-1`.
60+
pub resource: Vec<Number>,
5761
}
5862

5963
pub async fn samply_to_folded(loaded: FxProfile) -> String {
@@ -74,40 +78,36 @@ pub async fn samply_to_folded(loaded: FxProfile) -> String {
7478
);
7579
}
7680

77-
let mut folded_frames: BTreeMap<Vec<String>, u64> = BTreeMap::new();
81+
let mut folded_frames: BTreeMap<Vec<Option<String>>, u64> = BTreeMap::new();
7882
for thread in loaded.threads.into_iter().filter(|t| t.is_main_thread) {
79-
let mut frame_lookuped = vec![];
80-
for frame_id in 0..thread.frame_table.address.len() {
81-
let address = thread.frame_table.address[frame_id];
82-
let func_id = thread.frame_table.func[frame_id];
83-
let resource_id = thread.func_table.resource[func_id];
84-
let maybe_symbol_map = &symbol_maps[resource_id];
85-
86-
if let Some(symbols_map) = maybe_symbol_map {
87-
if let Some(lookuped) = symbols_map
83+
let frame_lookuped = join_all((0..thread.frame_table.address.len()).map(|frame_id| {
84+
let fr_address = &thread.frame_table.address;
85+
let fr_func = &thread.frame_table.func;
86+
let fn_resource = &thread.func_table.resource;
87+
let symbol_maps = &symbol_maps;
88+
async move {
89+
let address = fr_address[frame_id].as_u64()?;
90+
let func_id = fr_func[frame_id];
91+
let resource_id = fn_resource[func_id].as_u64()?;
92+
let symbol_map = symbol_maps[resource_id as usize].as_ref()?;
93+
let lookuped = symbol_map
8894
.lookup(LookupAddress::Relative(address as u32))
89-
.await
90-
{
91-
if let Some(inline_frames) = lookuped.frames {
92-
frame_lookuped.push(
93-
inline_frames
94-
.into_iter()
95-
.rev()
96-
.map(|inline| {
97-
inline.function.unwrap_or_else(|| "unknown".to_string())
98-
})
99-
.join(";"),
100-
);
101-
} else {
102-
frame_lookuped.push(lookuped.symbol.name);
103-
}
95+
.await?;
96+
97+
if let Some(inline_frames) = lookuped.frames {
98+
Some(
99+
inline_frames
100+
.into_iter()
101+
.rev()
102+
.map(|inline| inline.function.unwrap_or_else(|| "unknown".to_string()))
103+
.join(";"),
104+
)
104105
} else {
105-
frame_lookuped.push("unknown".to_string());
106+
Some(lookuped.symbol.name)
106107
}
107-
} else {
108-
frame_lookuped.push("unknown".to_string());
109108
}
110-
}
109+
}))
110+
.await;
111111

112112
let all_leaves_grouped = thread
113113
.samples
@@ -116,10 +116,10 @@ pub async fn samply_to_folded(loaded: FxProfile) -> String {
116116
.enumerate()
117117
.filter_map(|(idx, s)| s.map(|s| (idx, s)))
118118
.map(|(idx, leaf)| (leaf, thread.samples.weight[idx]))
119-
.chunk_by(|v| v.0)
119+
.chunk_by(|&(leaf, _)| leaf)
120120
.into_iter()
121121
.map(|(leaf, group)| {
122-
let weight = group.map(|t| t.1).sum();
122+
let weight = group.map(|(_leaf, weight)| weight).sum();
123123
(leaf, weight)
124124
})
125125
.collect::<Vec<(usize, u64)>>();
@@ -143,7 +143,7 @@ pub async fn samply_to_folded(loaded: FxProfile) -> String {
143143
if i != 0 {
144144
output.push(';');
145145
}
146-
output.push_str(s);
146+
output.push_str(s.as_deref().unwrap_or("unknown"));
147147
}
148148

149149
output.push_str(&format!(" {}\n", weight));

hydro_lang/src/rewrites/analyze_counter.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@ use crate::ir::*;
88
/// Returns (op_id, count)
99
pub fn parse_counter_usage(measurement: String) -> (usize, usize) {
1010
let regex = Regex::new(r"\((\d+)\): (\d+)").unwrap();
11-
let matches = regex.captures_iter(&measurement).last().unwrap();
11+
let matches = regex.captures_iter(&measurement).last().unwrap_or_else(|| {
12+
::core::panic!(
13+
"Failed to parse counter usage from measurement: {:?}",
14+
measurement
15+
)
16+
});
1217
let op_id = matches[1].parse::<usize>().unwrap();
1318
let count = matches[2].parse::<usize>().unwrap();
1419
(op_id, count)

0 commit comments

Comments
 (0)