Skip to content

Commit b5252d6

Browse files
committed
Refactor profiling configuration to use unified ProfilingConfig enum
Replace the confusing dual profiling configuration system (ProfilingStrategy + GuestProfileConfig) with a single, unified ProfilingConfig enum that cleanly distinguishes between native profiling (PerfMap/JitDump/VTune) and guest profiling (Firefox Profiler format).
1 parent 5600294 commit b5252d6

10 files changed

Lines changed: 110 additions & 85 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- Add stub implementations for bot detection hostcalls.
1212
- Add no-op implementations for stale-if-error hostcalls ([#591](https://github.com/fastly/Viceroy/pull/591))
1313
- Guest profiling support added for components. ([#593](https://github.com/fastly/Viceroy/pull/593))
14+
- Internal unification of native/guest profiling config. (#[597](https://github.com/fastly/Viceroy/pull/597))
1415

1516
## 0.16.4 (2026-01-26)
1617

cli/src/execute_ctx.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::time::Duration;
66
use tokio::time::timeout;
77
use tracing::{Level, Metadata, event};
88
use tracing_subscriber::fmt::writer::MakeWriter;
9-
use viceroy_lib::{BackendConnector, ExecuteCtx, GuestProfileConfig, config::FastlyConfig};
9+
use viceroy_lib::{BackendConnector, ExecuteCtx, config::FastlyConfig};
1010

1111
pub(crate) enum Stdio {
1212
Stdout(Stdout),
@@ -67,14 +67,12 @@ impl<'a> MakeWriter<'a> for StdWriter {
6767
pub(crate) async fn create_execution_context(
6868
args: &SharedArgs,
6969
check_backends: bool,
70-
guest_profile_config: Option<GuestProfileConfig>,
7170
) -> Result<Arc<ExecuteCtx>, anyhow::Error> {
7271
let input = args.input();
7372
let ctx = ExecuteCtx::build(
7473
input,
75-
args.profiling_strategy(),
74+
args.profiling_config(),
7675
args.wasi_modules(),
77-
guest_profile_config,
7876
args.unknown_import_behavior(),
7977
args.adapt(),
8078
)?

cli/src/opts.rs

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
33
use std::time::Duration;
44

5-
use viceroy_lib::{GuestProfileConfig, config::UnknownImportBehavior};
6-
75
use {
86
clap::{Args, Parser, Subcommand, ValueEnum},
97
std::net::{IpAddr, Ipv4Addr},
@@ -12,7 +10,11 @@ use {
1210
net::SocketAddr,
1311
path::{Path, PathBuf},
1412
},
15-
viceroy_lib::{Error, ProfilingStrategy, config::ExperimentalModule},
13+
viceroy_lib::{
14+
Error, ProfilingConfig,
15+
config::{ExperimentalModule, UnknownImportBehavior},
16+
},
17+
wasmtime::ProfilingStrategy,
1618
};
1719

1820
// Command-line arguments for the Viceroy CLI.
@@ -179,39 +181,30 @@ impl SharedArgs {
179181
self.log_stderr
180182
}
181183

182-
/// Whether to enable wasmtime's builtin profiler.
183-
pub fn profiling_strategy(&self) -> ProfilingStrategy {
184-
match self.profile {
185-
Some(Profile::Native(s)) => s,
186-
_ => ProfilingStrategy::None,
187-
}
188-
}
189-
190-
/// Port running local Pushpin proxy.
191-
pub fn local_pushpin_proxy_port(&self) -> Option<u16> {
192-
self.local_pushpin_proxy_port
193-
}
194-
195-
/// Configuration for guest profiling if enabled
196-
pub fn guest_profile_config(&self) -> Option<GuestProfileConfig> {
197-
if let Some(Profile::Guest {
198-
path,
199-
sample_period,
200-
}) = &self.profile
201-
{
202-
Some(GuestProfileConfig {
184+
/// Get the unified profiling configuration
185+
pub fn profiling_config(&self) -> ProfilingConfig {
186+
match &self.profile {
187+
Some(Profile::Native(strategy)) => ProfilingConfig::Native(*strategy),
188+
Some(Profile::Guest {
189+
path,
190+
sample_period,
191+
}) => ProfilingConfig::Guest {
203192
path: PathBuf::from(
204193
path.as_ref()
205194
.map(|p| p.as_str())
206195
.unwrap_or("guest-profiles"),
207196
),
208-
sample_period: sample_period.unwrap_or_else(|| Duration::from_micros(50)),
209-
})
210-
} else {
211-
None
197+
sample_period: sample_period.unwrap_or(Duration::from_micros(50)),
198+
},
199+
None => ProfilingConfig::None,
212200
}
213201
}
214202

203+
/// Port running local Pushpin proxy.
204+
pub fn local_pushpin_proxy_port(&self) -> Option<u16> {
205+
self.local_pushpin_proxy_port
206+
}
207+
215208
/// Set of experimental wasi modules to link against.
216209
pub fn wasi_modules(&self) -> HashSet<ExperimentalModule> {
217210
self.experimental_modules.iter().map(|x| x.into()).collect()

cli/src/subcommands/run.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,7 @@ pub(crate) async fn exec(run_args: RunArgs) -> ExitCode {
2424
/// Execute a Wasm program in the Viceroy environment.
2525
async fn run_wasm_main(run_args: RunArgs) -> Result<(), anyhow::Error> {
2626
// Load the wasm module into an execution context
27-
let ctx = create_execution_context(
28-
run_args.shared(),
29-
false,
30-
run_args.shared().guest_profile_config(),
31-
)
32-
.await?;
27+
let ctx = create_execution_context(run_args.shared(), false).await?;
3328
let input = run_args.shared().input();
3429
let program_name = match input.file_stem() {
3530
Some(stem) => stem.to_string_lossy(),

cli/src/subcommands/serve.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::opts::ServeArgs;
22
use crate::{create_execution_context, install_tracing_subscriber};
33
use std::process::ExitCode;
44
use tracing::{Level, event};
5-
use viceroy_lib::{Error, ViceroyService};
5+
use viceroy_lib::{Error, ProfilingConfig, ViceroyService};
66

77
pub(crate) async fn exec(serve_args: ServeArgs) -> ExitCode {
88
install_tracing_subscriber(serve_args.shared().verbosity());
@@ -29,15 +29,11 @@ pub(crate) async fn exec(serve_args: ServeArgs) -> ExitCode {
2929
/// Create a new server, bind it to an address, and serve responses until an error occurs.
3030
async fn serve(serve_args: ServeArgs) -> Result<(), Error> {
3131
// Load the wasm module into an execution context
32-
let ctx = create_execution_context(
33-
serve_args.shared(),
34-
true,
35-
serve_args.shared().guest_profile_config(),
36-
)
37-
.await?;
32+
let ctx = create_execution_context(serve_args.shared(), true).await?;
3833

39-
if let Some(guest_profile_config) = serve_args.shared().guest_profile_config() {
40-
std::fs::create_dir_all(guest_profile_config.path)?;
34+
// Create profile directory if guest profiling is enabled
35+
if let ProfilingConfig::Guest { path, .. } = serve_args.shared().profiling_config() {
36+
std::fs::create_dir_all(path)?;
4137
}
4238

4339
let addr = serve_args.addr();

cli/tests/integration/common.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use std::{
1414
use tracing_subscriber::filter::EnvFilter;
1515
use viceroy_lib::config::UnknownImportBehavior;
1616
use viceroy_lib::{
17-
ExecuteCtx, ProfilingStrategy, ViceroyService,
17+
ExecuteCtx, ProfilingConfig, ViceroyService,
1818
body::Body,
1919
config::{
2020
Acls, DeviceDetection, Dictionaries, FastlyConfig, Geolocation, ObjectStores, SecretStores,
@@ -92,8 +92,7 @@ pub struct Test {
9292
via_hyper: bool,
9393
unknown_import_behavior: UnknownImportBehavior,
9494
adapt_component: bool,
95-
profiling_strategy: ProfilingStrategy,
96-
guest_profile_config: Option<viceroy_lib::GuestProfileConfig>,
95+
profiling: ProfilingConfig,
9796
}
9897

9998
impl Test {
@@ -118,8 +117,7 @@ impl Test {
118117
via_hyper: false,
119118
unknown_import_behavior: Default::default(),
120119
adapt_component: false,
121-
profiling_strategy: ProfilingStrategy::None,
122-
guest_profile_config: None,
120+
profiling: ProfilingConfig::None,
123121
}
124122
}
125123

@@ -144,8 +142,7 @@ impl Test {
144142
via_hyper: false,
145143
unknown_import_behavior: Default::default(),
146144
adapt_component: false,
147-
profiling_strategy: ProfilingStrategy::None,
148-
guest_profile_config: None,
145+
profiling: ProfilingConfig::None,
149146
}
150147
}
151148

@@ -293,9 +290,9 @@ impl Test {
293290
self
294291
}
295292

296-
/// Enable guest profiling with the specified configuration.
297-
pub fn with_guest_profiling(mut self, config: viceroy_lib::GuestProfileConfig) -> Self {
298-
self.guest_profile_config = Some(config);
293+
/// Set the profiling configuration for this test.
294+
pub fn with_profiling(mut self, profiling: ProfilingConfig) -> Self {
295+
self.profiling = profiling;
299296
self
300297
}
301298

@@ -343,9 +340,8 @@ impl Test {
343340

344341
let ctx = ExecuteCtx::build(
345342
&self.module_path,
346-
self.profiling_strategy.clone(),
343+
self.profiling.clone(),
347344
HashSet::new(),
348-
self.guest_profile_config.clone(),
349345
self.unknown_import_behavior,
350346
self.adapt_component,
351347
)?

cli/tests/integration/profiling.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use {
77
},
88
hyper::StatusCode,
99
std::time::Duration,
10-
viceroy_lib::GuestProfileConfig,
10+
viceroy_lib::ProfilingConfig,
1111
};
1212

1313
viceroy_test!(guest_profiling_works, |is_component| {
@@ -18,7 +18,7 @@ viceroy_test!(guest_profiling_works, |is_component| {
1818

1919
let resp = Test::using_fixture("noop.wasm")
2020
.adapt_component(is_component)
21-
.with_guest_profiling(GuestProfileConfig {
21+
.with_profiling(ProfilingConfig::Guest {
2222
path: profile_dir.clone(),
2323
sample_period: Duration::from_micros(50),
2424
})

src/execute.rs

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,56 @@ impl Instance {
7676
}
7777
}
7878

79+
/// Configuration for profiling Wasm execution.
80+
///
81+
/// Viceroy supports two types of profiling:
82+
/// - **Guest profiling**: Profiles the WebAssembly guest code itself, producing Firefox profiler compatible JSON.
83+
/// - **Native profiling**: Profiles wasmtime's JIT compiler for debugging across guest, viceroy and wasmtime.
7984
#[derive(Clone)]
80-
pub struct GuestProfileConfig {
81-
/// Path to write profiling results from the guest. In serve mode,
82-
/// this must refer to a directory, while in run mode it names
83-
/// a file.
84-
pub path: PathBuf,
85-
/// Period at which the guest should be profiled.
86-
pub sample_period: Duration,
85+
pub enum ProfilingConfig {
86+
/// No profiling enabled
87+
None,
88+
/// Profile the WebAssembly guest code
89+
Guest {
90+
/// Path to write profiling results. In serve mode, this should be a directory
91+
/// where per-request profiles are written. In run mode, this is a single file.
92+
path: PathBuf,
93+
/// Period at which the guest should be sampled (default: 50μs)
94+
sample_period: Duration,
95+
},
96+
/// Profile wasmtime's JIT compiler using native profiling tools
97+
Native(wasmtime::ProfilingStrategy),
98+
}
99+
100+
impl ProfilingConfig {
101+
/// Get the native profiling strategy for wasmtime
102+
pub fn native_strategy(&self) -> wasmtime::ProfilingStrategy {
103+
match self {
104+
ProfilingConfig::Native(strategy) => *strategy,
105+
_ => wasmtime::ProfilingStrategy::None,
106+
}
107+
}
108+
109+
/// Get the guest profile configuration if guest profiling is enabled
110+
fn guest_config(&self) -> Option<GuestProfileConfig> {
111+
match self {
112+
ProfilingConfig::Guest {
113+
path,
114+
sample_period,
115+
} => Some(GuestProfileConfig {
116+
path: path.clone(),
117+
sample_period: *sample_period,
118+
}),
119+
_ => None,
120+
}
121+
}
122+
}
123+
124+
// Keep GuestProfileConfig internal for now to maintain backwards compatibility
125+
#[derive(Clone)]
126+
struct GuestProfileConfig {
127+
path: PathBuf,
128+
sample_period: Duration,
87129
}
88130

89131
pub struct NextRequest(Option<(DownstreamRequest, Arc<ExecuteCtx>)>);
@@ -165,12 +207,11 @@ pub struct ExecuteCtx {
165207
}
166208

167209
impl ExecuteCtx {
168-
/// Build a new execution context, given the path to a module and a set of experimental wasi modules.
210+
/// Build a new execution context with unified profiling configuration.
169211
pub fn build(
170212
module_path: impl AsRef<Path>,
171-
profiling_strategy: ProfilingStrategy,
213+
profiling: ProfilingConfig,
172214
wasi_modules: HashSet<ExperimentalModule>,
173-
guest_profile_config: Option<GuestProfileConfig>,
174215
unknown_import_behavior: UnknownImportBehavior,
175216
adapt_components: bool,
176217
) -> Result<ExecuteCtxBuilder, Error> {
@@ -200,7 +241,7 @@ impl ExecuteCtx {
200241
(is_wat, is_component, input)
201242
};
202243

203-
let config = &configure_wasmtime(is_component, profiling_strategy);
244+
let config = &configure_wasmtime(is_component, profiling.native_strategy());
204245
let engine = Engine::new(config)?;
205246
let instance_pre = if is_component {
206247
warn!(
@@ -278,7 +319,8 @@ impl ExecuteCtx {
278319
let epoch_increment_stop = Arc::new(AtomicBool::new(false));
279320
let engine_clone = engine.clone();
280321
let epoch_increment_stop_clone = epoch_increment_stop.clone();
281-
let sample_period = guest_profile_config
322+
let sample_period = profiling
323+
.guest_config()
282324
.as_ref()
283325
.map(|c| c.sample_period)
284326
.unwrap_or(DEFAULT_EPOCH_INTERRUPTION_PERIOD);
@@ -309,28 +351,28 @@ impl ExecuteCtx {
309351
shielding_sites: ShieldingSites::new(),
310352
epoch_increment_thread,
311353
epoch_increment_stop,
312-
guest_profile_config: guest_profile_config.map(|c| Arc::new(c)),
354+
guest_profile_config: profiling.guest_config().map(Arc::new),
313355
cache: Arc::new(Cache::default()),
314356
pending_reuse: Arc::new(AsyncMutex::new(vec![])),
315357
};
316358

317359
Ok(ExecuteCtxBuilder { inner })
318360
}
319361

320-
/// Create a new execution context, given the path to a module and a set of experimental wasi modules.
362+
/// Create a new execution context with unified profiling configuration.
363+
///
364+
/// This is a convenience wrapper around `build().finish()`.
321365
pub fn new(
322366
module_path: impl AsRef<Path>,
323-
profiling_strategy: ProfilingStrategy,
367+
profiling: ProfilingConfig,
324368
wasi_modules: HashSet<ExperimentalModule>,
325-
guest_profile_config: Option<GuestProfileConfig>,
326369
unknown_import_behavior: UnknownImportBehavior,
327370
adapt_components: bool,
328371
) -> Result<Arc<Self>, Error> {
329372
ExecuteCtx::build(
330373
module_path,
331-
profiling_strategy,
374+
profiling,
332375
wasi_modules,
333-
guest_profile_config,
334376
unknown_import_behavior,
335377
adapt_components,
336378
)?
@@ -415,11 +457,12 @@ impl ExecuteCtx {
415457
/// ```no_run
416458
/// # use std::collections::HashSet;
417459
/// use hyper::{Body, http::Request};
418-
/// # use viceroy_lib::{Error, ExecuteCtx, ProfilingStrategy, ViceroyService};
460+
/// # use viceroy_lib::{Error, ExecuteCtx, ProfilingConfig, ViceroyService};
461+
/// # use viceroy_lib::config::UnknownImportBehavior;
419462
/// # async fn f() -> Result<(), Error> {
420463
/// # let req = Request::new(Body::from(""));
421464
/// let adapt_core_wasm = false;
422-
/// let ctx = ExecuteCtx::new("path/to/a/file.wasm", ProfilingStrategy::None, HashSet::new(), None, Default::default(), adapt_core_wasm)?;
465+
/// let ctx = ExecuteCtx::new("path/to/a/file.wasm", ProfilingConfig::None, HashSet::new(), UnknownImportBehavior::LinkError, adapt_core_wasm)?;
423466
/// let local = "127.0.0.1:80".parse().unwrap();
424467
/// let remote = "127.0.0.1:0".parse().unwrap();
425468
/// let resp = ctx.handle_request(req, local, remote).await?;

src/lib.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ mod upstream;
4242
pub mod wiggle_abi;
4343

4444
pub use {
45-
error::Error, execute::ExecuteCtx, execute::GuestProfileConfig, service::ViceroyService,
46-
upstream::BackendConnector, wasmtime::ProfilingStrategy,
45+
error::Error,
46+
execute::{ExecuteCtx, ProfilingConfig},
47+
service::ViceroyService,
48+
upstream::BackendConnector,
4749
};

0 commit comments

Comments
 (0)