Skip to content

Commit

Permalink
refactor(profiler): remove from optimized builds
Browse files Browse the repository at this point in the history
While the overhead of profiling is so small that it is difficult to
measure, an optimized build should not be slowed down. Remove the
corresponding code from optimized builds using conditional compilation.

It is possible to opt-in to profiling by disabling `default-features`.
  • Loading branch information
jan-ferdinand committed May 11, 2024
1 parent 1ecd11c commit f434015
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 7 deletions.
4 changes: 4 additions & 0 deletions triton-vm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ proptest-arbitrary-interop.workspace = true
serde_json.workspace = true
test-strategy.workspace = true

[features]
default = ["no_profile"]
no_profile = [] # see `profiler.rs` for an explanation of this seemingly backwards feature

[lints]
workspace = true

Expand Down
67 changes: 60 additions & 7 deletions triton-vm/src/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,36 @@
//! This allows multiple threads to run in parallel without interfering with
//! each other's profiling data.
//!
//! # Enabling Profiling
//!
//! In release builds, profiling is disabled by default to allow for the fastest
//! possible proof generation. To enable profiling, either make sure that
//! `debug_assertions` is set, or add the following to your `Cargo.toml`:
//!
//! ```toml
//! [dependencies]
//! triton-vm = { version = "x.y.z", default-features = false }
//! ```
//!
//! ### A note on the `no_profile` feature design decision
//!
//! The feature `no_profile` _disables_ profiling, and is enabled by default.
//! This seems backwards. However, it is an expression of how Triton VM favors
//! performance over profiling. Note [how Cargo resolves dependencies][deps]:
//! if some dependency is transitively declared multiple times, the _union_ of
//! all features will be enabled.
//!
//! Imagine some dependency `foo` enables a hypothetical `do_profile` feature.
//! If another dependency `bar` requires the most efficient proof generation,
//! it would be slowed down by `foo` and could do nothing about it. Instead,
//! Disabling profiling by <i>en</i>abling the feature `no_profile` allows `bar`
//! to dictate. This:
//! 1. makes the profile reports of `foo` disappear, which is sad, but
//! 1. lets `bar` be fast, which is more important for Triton VM.
//!
//! [proving]: crate::stark::Stark::prove
//! [verifying]: crate::stark::Stark::verify
//! [deps]: https://doc.rust-lang.org/cargo/reference/features.html#feature-unification
use std::cell::RefCell;
use std::cmp::max;
Expand Down Expand Up @@ -38,17 +66,27 @@ thread_local! {

/// Start profiling. If the profiler is already running, this function cancels
/// the current profiling session and starts a new one.
///
/// See the module-level documentation for information on how to enable profiling.
pub fn start(profile_name: impl Into<String>) {
PROFILER.replace(Some(TritonProfiler::new(profile_name)));
if cfg!(any(debug_assertions, not(feature = "no_profile"))) {
PROFILER.replace(Some(TritonProfiler::new(profile_name)));
}
}

/// Stop the current profiling session and generate a [`Report`]. If the
/// profiler is not running, an empty [`Report`] is returned.
/// profiler is disabled or not running, an empty [`Report`] is returned.
///
/// See the module-level documentation for information on how to enable profiling.
pub fn finish() -> Report {
PROFILER
.take()
.map(|mut profiler| profiler.report())
.unwrap_or_default()
if cfg!(any(debug_assertions, not(feature = "no_profile"))) {
PROFILER
.take()
.map(|mut profiler| profiler.report())
.unwrap_or_default()
} else {
Report::default()
}
}

#[derive(Debug, Clone, Eq, PartialEq, Hash)]
Expand Down Expand Up @@ -92,6 +130,7 @@ impl TritonProfiler {
}
}

#[cfg(any(debug_assertions, not(feature = "no_profile")))]
fn ignoring(&self) -> bool {
self.stack
.last()
Expand Down Expand Up @@ -205,12 +244,14 @@ impl TritonProfiler {
}
}

#[cfg(any(debug_assertions, not(feature = "no_profile")))]
pub fn start(&mut self, name: &str, category: Option<String>) {
if !self.ignoring() {
self.plain_start(name, TaskType::Generic, category);
}
}

#[cfg(any(debug_assertions, not(feature = "no_profile")))]
fn plain_start(
&mut self,
name: impl Into<String>,
Expand All @@ -235,6 +276,7 @@ impl TritonProfiler {
});
}

#[cfg(any(debug_assertions, not(feature = "no_profile")))]
pub fn iteration_zero(&mut self, name: &str, category: Option<String>) {
if self.ignoring() {
return;
Expand Down Expand Up @@ -297,6 +339,7 @@ impl TritonProfiler {
}
}

#[cfg(any(debug_assertions, not(feature = "no_profile")))]
pub fn stop(&mut self, name: &str) {
assert!(
!self.stack.is_empty(),
Expand Down Expand Up @@ -423,7 +466,12 @@ impl Report {

impl Default for Report {
fn default() -> Self {
let name = "__empty__".to_string();
let name = if cfg!(feature = "no_profile") {
"Triton VM's profiler is disabled through feature `no_profile`.".to_string()
} else {
"__empty__".to_string()
};

Self {
name,
tasks: vec![],
Expand Down Expand Up @@ -583,13 +631,15 @@ impl Display for Report {
/// The second, optional argument is a task category.
macro_rules! prof_start {
($s:expr, $c:expr) => {{
#[cfg(any(debug_assertions, not(feature = "no_profile")))]
crate::profiler::PROFILER.with_borrow_mut(|profiler| {
if let Some(profiler) = profiler.as_mut() {
profiler.start($s, Some($c.to_string()));
}
})
}};
($s:expr) => {{
#[cfg(any(debug_assertions, not(feature = "no_profile")))]
crate::profiler::PROFILER.with_borrow_mut(|profiler| {
if let Some(profiler) = profiler.as_mut() {
profiler.start($s, None);
Expand All @@ -606,6 +656,7 @@ pub(crate) use prof_start;
/// match to prevent the accidental stopping of a different task.
macro_rules! prof_stop {
($s:expr) => {{
#[cfg(any(debug_assertions, not(feature = "no_profile")))]
crate::profiler::PROFILER.with_borrow_mut(|profiler| {
if let Some(profiler) = profiler.as_mut() {
profiler.stop($s);
Expand All @@ -622,13 +673,15 @@ pub(crate) use prof_stop;
/// the loop has to be stopped with [`prof_stop`] after the loop.
macro_rules! prof_itr0 {
($s:expr, $c:expr) => {{
#[cfg(any(debug_assertions, not(feature = "no_profile")))]
crate::profiler::PROFILER.with_borrow_mut(|profiler| {
if let Some(profiler) = profiler.as_mut() {
profiler.iteration_zero($s, Some($c.to_string()));
}
})
}};
($s:expr) => {{
#[cfg(any(debug_assertions, not(feature = "no_profile")))]
crate::profiler::PROFILER.with_borrow_mut(|profiler| {
if let Some(profiler) = profiler.as_mut() {
profiler.iteration_zero($s, None);
Expand Down

0 comments on commit f434015

Please sign in to comment.