From 2b37e25f7bb8d5be9803e876771d3adf807fbe0e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 08:28:38 +0200 Subject: [PATCH 1/8] refactor * reorganize package * properly fix clippy warning for when tracing is disabled. --- .cargo/config.toml | 1 - gix-trace/src/disabled.rs | 29 +++ gix-trace/src/enabled.rs | 322 ++++++++++++++++++++++++++++++++++ gix-trace/src/lib.rs | 360 +------------------------------------- 4 files changed, 358 insertions(+), 354 deletions(-) create mode 100644 gix-trace/src/disabled.rs create mode 100644 gix-trace/src/enabled.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index fd39b0dbed6..3e6810586b8 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -2,7 +2,6 @@ rustflags = [ # Rustc lints # "-W", "warning_name" - "-A", "clippy::let_unit_value", # in 'small' builds this triggers as the `span!` macro yields `let x = ()`. No way to prevent it in macro apparently. # Clippy lints "-W", "clippy::cloned_instead_of_copied", diff --git a/gix-trace/src/disabled.rs b/gix-trace/src/disabled.rs new file mode 100644 index 00000000000..0dfea52bae5 --- /dev/null +++ b/gix-trace/src/disabled.rs @@ -0,0 +1,29 @@ +/// A workaround for a clippy warning +#[doc(hidden)] +pub struct Unit; + +/// A macro to create a span. +#[macro_export] +macro_rules! span { + (target: $target:expr, $lvl:expr, $name:expr, $($fields:tt)*) => { + $crate::Unit + }; + (target: $target:expr, $lvl:expr, $name:expr) => { + $crate::span!(target: $target, $lvl, $name,) + }; + ($lvl:expr, $name:expr, $($fields:tt)*) => { + $crate::span!( + target: module_path!(), + $lvl, + $name, + $($fields)* + ) + }; + ($lvl:expr, $name:expr) => { + $crate::span!( + target: module_path!(), + $lvl, + $name, + ) + }; +} diff --git a/gix-trace/src/enabled.rs b/gix-trace/src/enabled.rs new file mode 100644 index 00000000000..c154b0399b2 --- /dev/null +++ b/gix-trace/src/enabled.rs @@ -0,0 +1,322 @@ +use tracing_core::{dispatcher::get_default as with_dispatcher, span::Id, Dispatch}; + +// these are used later in macros. +pub use tracing_core::{field, metadata, Metadata}; + +/// An entered span which will exit on drop. +pub struct Span { + id: Option<(Id, Dispatch)>, +} + +impl Span { + #[allow(missing_docs)] + pub fn new( + level: crate::Level, + meta: &'static Metadata<'static>, + values: &tracing_core::field::ValueSet<'_>, + ) -> Self { + if level > crate::MAX_LEVEL { + Self { id: None } + } else { + with_dispatcher(|dispatch| { + let id = dispatch.new_span(&tracing_core::span::Attributes::new(meta, values)); + dispatch.enter(&id); + Self { + id: Some((id, dispatch.clone())), + } + }) + } + } +} + +impl Drop for Span { + fn drop(&mut self) { + if let Some((id, dispatch)) = self.id.take() { + dispatch.exit(&id); + dispatch.try_close(id); + } + } +} + +#[doc(hidden)] +pub struct MetaOnlyCallsite(pub &'static Metadata<'static>); + +impl tracing_core::callsite::Callsite for MetaOnlyCallsite { + fn set_interest(&self, _: tracing_core::subscriber::Interest) {} + + fn metadata(&self) -> &Metadata<'_> { + self.0 + } +} + +#[doc(hidden)] +impl crate::Level { + pub const fn into_tracing_level(self) -> tracing_core::Level { + match self { + crate::Level::Coarse => tracing_core::Level::INFO, + crate::Level::Detail => tracing_core::Level::DEBUG, + } + } +} + +/// A macro to create a span. +#[cfg(feature = "tracing")] +#[macro_export] +macro_rules! span { + (target: $target:expr, $lvl:expr, $name:expr, $($fields:tt)*) => { + { + static META: $crate::Metadata<'static> = { + $crate::metadata! { + name: $name, + target: $target, + level: $lvl.into_tracing_level(), + fields: $crate::fieldset!( $($fields)* ), + callsite: &$crate::MetaOnlyCallsite(&META), + kind: $crate::metadata::Kind::SPAN, + } + }; + + $crate::Span::new( + $lvl, + &META, + &$crate::valueset!(META.fields(), $($fields)*), + ) + } + }; + (target: $target:expr, $lvl:expr, $name:expr) => { + $crate::span!(target: $target, $lvl, $name,) + }; + ($lvl:expr, $name:expr, $($fields:tt)*) => { + $crate::span!( + target: module_path!(), + $lvl, + $name, + $($fields)* + ) + }; + ($lvl:expr, $name:expr) => { + $crate::span!( + target: module_path!(), + $lvl, + $name, + ) + }; +} + +// Copied from`tracing`, would be nice to have it in `tracing-core`. +#[doc(hidden)] +#[macro_export] +macro_rules! fieldset { + // == base case == + (@ { $(,)* $($out:expr),* $(,)* } $(,)*) => { + &[ $($out),* ] + }; + + // == recursive cases (more tts) == + (@ { $(,)* $($out:expr),* } $($k:ident).+ = ?$val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } $($k:ident).+ = %$val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } $($k:ident).+ = $val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) + }; + // TODO(#1138): determine a new syntax for uninitialized span fields, and + // re-enable this. + // (@ { $($out:expr),* } $($k:ident).+ = _, $($rest:tt)*) => { + // $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) + // }; + (@ { $(,)* $($out:expr),* } ?$($k:ident).+, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } %$($k:ident).+, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } $($k:ident).+, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) + }; + + // Handle literal names + (@ { $(,)* $($out:expr),* } $k:literal = ?$val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, $k } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } $k:literal = %$val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, $k } $($rest)*) + }; + (@ { $(,)* $($out:expr),* } $k:literal = $val:expr, $($rest:tt)*) => { + $crate::fieldset!(@ { $($out),*, $k } $($rest)*) + }; + + // Remainder is unparseable, but exists --- must be format args! + (@ { $(,)* $($out:expr),* } $($rest:tt)+) => { + $crate::fieldset!(@ { "message", $($out),*, }) + }; + + // == entry == + ($($args:tt)*) => { + $crate::fieldset!(@ { } $($args)*,) + }; +} + +// Copied from`tracing`, would be nice to have it in `tracing-core`. +#[doc(hidden)] +#[macro_export] +macro_rules! valueset { + + // === base case === + (@ { $(,)* $($val:expr),* $(,)* }, $next:expr $(,)*) => { + &[ $($val),* ] + }; + + // === recursive case (more tts) === + + // TODO(#1138): determine a new syntax for uninitialized span fields, and + // re-enable this. + // (@{ $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = _, $($rest:tt)*) => { + // $crate::valueset!(@ { $($out),*, (&$next, None) }, $next, $($rest)*) + // }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = ?$val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = %$val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = $val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$($k).+ as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, ?$($k:ident).+, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$($k).+) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, %$($k:ident).+, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$($k).+) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = ?$val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = %$val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = $val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$($k).+ as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, ?$($k:ident).+) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$($k).+) as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, %$($k:ident).+) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$($k).+) as &dyn Value)) }, + $next, + ) + }; + + // Handle literal names + (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = ?$val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = %$val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = $val:expr, $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, + $next, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = ?$val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = %$val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, + $next, + ) + }; + (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = $val:expr) => { + $crate::valueset!( + @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, + $next, + ) + }; + + // Remainder is unparsable, but exists --- must be format args! + (@ { $(,)* $($out:expr),* }, $next:expr, $($rest:tt)+) => { + $crate::valueset!(@ { (&$next, Some(&format_args!($($rest)+) as &dyn Value)), $($out),* }, $next, ) + }; + + // === entry === + ($fields:expr, $($kvs:tt)+) => { + { + #[allow(unused_imports)] + use $crate::field::{debug, display, Value}; + let mut iter = $fields.iter(); + $fields.value_set($crate::valueset!( + @ { }, + iter.next().expect("FieldSet corrupted (this is a bug)"), + $($kvs)+ + )) + } + }; + ($fields:expr,) => { + { + $fields.value_set(&[]) + } + }; +} diff --git a/gix-trace/src/lib.rs b/gix-trace/src/lib.rs index 08b4a914476..aed7858beaa 100644 --- a/gix-trace/src/lib.rs +++ b/gix-trace/src/lib.rs @@ -32,118 +32,19 @@ pub const MAX_LEVEL: Level = Level::Detail; pub const MAX_LEVEL: Level = Level::Coarse; #[cfg(feature = "tracing")] -mod enabled { - use tracing_core::{dispatcher::get_default as with_dispatcher, span::Id, Dispatch}; +mod enabled; - // these are used later in macros. - pub use tracing_core::{field, metadata, Metadata}; - - /// An entered span which will exit on drop. - pub struct Span { - id: Option<(Id, Dispatch)>, - } - - impl Span { - #[allow(missing_docs)] - pub fn new( - level: crate::Level, - meta: &'static Metadata<'static>, - values: &tracing_core::field::ValueSet<'_>, - ) -> Self { - if level > crate::MAX_LEVEL { - Self { id: None } - } else { - with_dispatcher(|dispatch| { - let id = dispatch.new_span(&tracing_core::span::Attributes::new(meta, values)); - dispatch.enter(&id); - Self { - id: Some((id, dispatch.clone())), - } - }) - } - } - } - - impl Drop for Span { - fn drop(&mut self) { - if let Some((id, dispatch)) = self.id.take() { - dispatch.exit(&id); - dispatch.try_close(id); - } - } - } - - #[doc(hidden)] - pub struct MetaOnlyCallsite(pub &'static Metadata<'static>); - - impl tracing_core::callsite::Callsite for MetaOnlyCallsite { - fn set_interest(&self, _: tracing_core::subscriber::Interest) {} - - fn metadata(&self) -> &Metadata<'_> { - self.0 - } - } - - #[doc(hidden)] - impl crate::Level { - pub const fn into_tracing_level(self) -> tracing_core::Level { - match self { - crate::Level::Coarse => tracing_core::Level::INFO, - crate::Level::Detail => tracing_core::Level::DEBUG, - } - } - } -} #[cfg(feature = "tracing")] -pub use enabled::{MetaOnlyCallsite, Span}; +pub use enabled::Span; #[cfg(feature = "tracing")] #[doc(hidden)] -pub use enabled::{field, metadata, Metadata}; +pub use enabled::{field, metadata, MetaOnlyCallsite, Metadata}; -/// A macro to create a span. -#[cfg(feature = "tracing")] -#[macro_export] -macro_rules! span { - (target: $target:expr, $lvl:expr, $name:expr, $($fields:tt)*) => { - { - static META: $crate::Metadata<'static> = { - $crate::metadata! { - name: $name, - target: $target, - level: $lvl.into_tracing_level(), - fields: $crate::fieldset!( $($fields)* ), - callsite: &$crate::MetaOnlyCallsite(&META), - kind: $crate::metadata::Kind::SPAN, - } - }; - - $crate::Span::new( - $lvl, - &META, - &$crate::valueset!(META.fields(), $($fields)*), - ) - } - }; - (target: $target:expr, $lvl:expr, $name:expr) => { - $crate::span!(target: $target, $lvl, $name,) - }; - ($lvl:expr, $name:expr, $($fields:tt)*) => { - $crate::span!( - target: module_path!(), - $lvl, - $name, - $($fields)* - ) - }; - ($lvl:expr, $name:expr) => { - $crate::span!( - target: module_path!(), - $lvl, - $name, - ) - }; -} +#[cfg(not(feature = "tracing"))] +mod disabled; +#[cfg(not(feature = "tracing"))] +pub use disabled::Unit; /// Create a new [coarse][Level::Coarse] span. #[macro_export] @@ -194,250 +95,3 @@ macro_rules! detail { }; ($name:expr) => {$crate::coarse!($name,)}; } - -// Copied from`tracing`, would be nice to have it in `tracing-core`. -#[cfg(feature = "tracing")] -#[doc(hidden)] -#[macro_export] -macro_rules! fieldset { - // == base case == - (@ { $(,)* $($out:expr),* $(,)* } $(,)*) => { - &[ $($out),* ] - }; - - // == recursive cases (more tts) == - (@ { $(,)* $($out:expr),* } $($k:ident).+ = ?$val:expr, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) - }; - (@ { $(,)* $($out:expr),* } $($k:ident).+ = %$val:expr, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) - }; - (@ { $(,)* $($out:expr),* } $($k:ident).+ = $val:expr, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) - }; - // TODO(#1138): determine a new syntax for uninitialized span fields, and - // re-enable this. - // (@ { $($out:expr),* } $($k:ident).+ = _, $($rest:tt)*) => { - // $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) - // }; - (@ { $(,)* $($out:expr),* } ?$($k:ident).+, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) - }; - (@ { $(,)* $($out:expr),* } %$($k:ident).+, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) - }; - (@ { $(,)* $($out:expr),* } $($k:ident).+, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, stringify!($($k).+) } $($rest)*) - }; - - // Handle literal names - (@ { $(,)* $($out:expr),* } $k:literal = ?$val:expr, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, $k } $($rest)*) - }; - (@ { $(,)* $($out:expr),* } $k:literal = %$val:expr, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, $k } $($rest)*) - }; - (@ { $(,)* $($out:expr),* } $k:literal = $val:expr, $($rest:tt)*) => { - $crate::fieldset!(@ { $($out),*, $k } $($rest)*) - }; - - // Remainder is unparseable, but exists --- must be format args! - (@ { $(,)* $($out:expr),* } $($rest:tt)+) => { - $crate::fieldset!(@ { "message", $($out),*, }) - }; - - // == entry == - ($($args:tt)*) => { - $crate::fieldset!(@ { } $($args)*,) - }; -} - -// Copied from`tracing`, would be nice to have it in `tracing-core`. -#[cfg(feature = "tracing")] -#[doc(hidden)] -#[macro_export] -macro_rules! valueset { - - // === base case === - (@ { $(,)* $($val:expr),* $(,)* }, $next:expr $(,)*) => { - &[ $($val),* ] - }; - - // === recursive case (more tts) === - - // TODO(#1138): determine a new syntax for uninitialized span fields, and - // re-enable this. - // (@{ $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = _, $($rest:tt)*) => { - // $crate::valueset!(@ { $($out),*, (&$next, None) }, $next, $($rest)*) - // }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = ?$val:expr, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = %$val:expr, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = $val:expr, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&$($k).+ as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, ?$($k:ident).+, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&debug(&$($k).+) as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, %$($k:ident).+, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&display(&$($k).+) as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = ?$val:expr) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, - $next, - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = %$val:expr) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, - $next, - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+ = $val:expr) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, - $next, - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $($k:ident).+) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&$($k).+ as &dyn Value)) }, - $next, - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, ?$($k:ident).+) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&debug(&$($k).+) as &dyn Value)) }, - $next, - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, %$($k:ident).+) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&display(&$($k).+) as &dyn Value)) }, - $next, - ) - }; - - // Handle literal names - (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = ?$val:expr, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = %$val:expr, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = $val:expr, $($rest:tt)*) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, - $next, - $($rest)* - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = ?$val:expr) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&debug(&$val) as &dyn Value)) }, - $next, - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = %$val:expr) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&display(&$val) as &dyn Value)) }, - $next, - ) - }; - (@ { $(,)* $($out:expr),* }, $next:expr, $k:literal = $val:expr) => { - $crate::valueset!( - @ { $($out),*, (&$next, Some(&$val as &dyn Value)) }, - $next, - ) - }; - - // Remainder is unparsable, but exists --- must be format args! - (@ { $(,)* $($out:expr),* }, $next:expr, $($rest:tt)+) => { - $crate::valueset!(@ { (&$next, Some(&format_args!($($rest)+) as &dyn Value)), $($out),* }, $next, ) - }; - - // === entry === - ($fields:expr, $($kvs:tt)+) => { - { - #[allow(unused_imports)] - use $crate::field::{debug, display, Value}; - let mut iter = $fields.iter(); - $fields.value_set($crate::valueset!( - @ { }, - iter.next().expect("FieldSet corrupted (this is a bug)"), - $($kvs)+ - )) - } - }; - ($fields:expr,) => { - { - $fields.value_set(&[]) - } - }; -} - -/// A macro to create a span. -#[cfg(not(feature = "tracing"))] -#[macro_export] -macro_rules! span { - (target: $target:expr, $lvl:expr, $name:expr, $($fields:tt)*) => { - () - }; - (target: $target:expr, $lvl:expr, $name:expr) => { - $crate::span!(target: $target, $lvl, $name,) - }; - ($lvl:expr, $name:expr, $($fields:tt)*) => { - $crate::span!( - target: module_path!(), - $lvl, - $name, - $($fields)* - ) - }; - ($lvl:expr, $name:expr) => { - $crate::span!( - target: module_path!(), - $lvl, - $name, - ) - }; -} From aa9dc8f09826790d1259140f6026ff116d943ac1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 10:05:56 +0200 Subject: [PATCH 2/8] feat: add `Span::into_scope()` A way to conveniently auto-drop a span after executing a closure. --- gix-trace/src/disabled.rs | 5 +++-- gix-trace/src/enabled.rs | 1 + gix-trace/src/lib.rs | 9 ++++++++- gix-trace/tests/trace.rs | 3 ++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/gix-trace/src/disabled.rs b/gix-trace/src/disabled.rs index 0dfea52bae5..f07a5bdeef5 100644 --- a/gix-trace/src/disabled.rs +++ b/gix-trace/src/disabled.rs @@ -1,12 +1,13 @@ /// A workaround for a clippy warning #[doc(hidden)] -pub struct Unit; +#[derive(Clone)] +pub struct Span; /// A macro to create a span. #[macro_export] macro_rules! span { (target: $target:expr, $lvl:expr, $name:expr, $($fields:tt)*) => { - $crate::Unit + $crate::Span }; (target: $target:expr, $lvl:expr, $name:expr) => { $crate::span!(target: $target, $lvl, $name,) diff --git a/gix-trace/src/enabled.rs b/gix-trace/src/enabled.rs index c154b0399b2..6dab16e6289 100644 --- a/gix-trace/src/enabled.rs +++ b/gix-trace/src/enabled.rs @@ -4,6 +4,7 @@ use tracing_core::{dispatcher::get_default as with_dispatcher, span::Id, Dispatc pub use tracing_core::{field, metadata, Metadata}; /// An entered span which will exit on drop. +#[derive(Clone)] pub struct Span { id: Option<(Id, Dispatch)>, } diff --git a/gix-trace/src/lib.rs b/gix-trace/src/lib.rs index aed7858beaa..5ba216efaa6 100644 --- a/gix-trace/src/lib.rs +++ b/gix-trace/src/lib.rs @@ -37,6 +37,13 @@ mod enabled; #[cfg(feature = "tracing")] pub use enabled::Span; +impl Span { + /// Execute `f` in with this span active, consuming it. + pub fn into_scope(self, f: impl FnOnce() -> T) -> T { + f() + } +} + #[cfg(feature = "tracing")] #[doc(hidden)] pub use enabled::{field, metadata, MetaOnlyCallsite, Metadata}; @@ -44,7 +51,7 @@ pub use enabled::{field, metadata, MetaOnlyCallsite, Metadata}; #[cfg(not(feature = "tracing"))] mod disabled; #[cfg(not(feature = "tracing"))] -pub use disabled::Unit; +pub use disabled::Span; /// Create a new [coarse][Level::Coarse] span. #[macro_export] diff --git a/gix-trace/tests/trace.rs b/gix-trace/tests/trace.rs index cda2cbc712a..5ecdab54c68 100644 --- a/gix-trace/tests/trace.rs +++ b/gix-trace/tests/trace.rs @@ -2,7 +2,8 @@ use gix_trace::{coarse, detail, span}; #[test] fn span() { let _x = span!(gix_trace::Level::Coarse, "hello"); - span!(gix_trace::Level::Coarse, "hello", x = "value", y = 42); + let fourty_two = span!(gix_trace::Level::Coarse, "hello", x = "value", y = 42).into_scope(|| 42); + assert_eq!(fourty_two, 42); span!(target: "other", gix_trace::Level::Coarse, "hello", x = "value", y = 42); } From bd6e3d7682d122997f8d3543eb87a62121ef0669 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 10:06:26 +0200 Subject: [PATCH 3/8] adapt to changes in `gix-trace` --- src/shared.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/shared.rs b/src/shared.rs index fb0ed271413..8b5e640a4fe 100644 --- a/src/shared.rs +++ b/src/shared.rs @@ -165,10 +165,8 @@ pub mod pretty { let mut out = Vec::::new(); let mut err = Vec::::new(); - let span = gix::trace::coarse!("run"); - let res = run(progress::DoOrDiscard::from(Some(sub_progress)), &mut out, &mut err); - #[cfg(feature = "tracing")] - drop(span); + let res = gix::trace::coarse!("run") + .into_scope(|| run(progress::DoOrDiscard::from(Some(sub_progress)), &mut out, &mut err)); handle.shutdown_and_wait(); std::io::Write::write_all(&mut stdout(), &out)?; @@ -219,10 +217,9 @@ pub mod pretty { // We might have something interesting to show, which would be hidden by the alternate screen if there is a progress TUI // We know that the printing happens at the end, so this is fine. let mut out = Vec::new(); - let span = gix::trace::coarse!("run", name = name); - let res = run(progress::DoOrDiscard::from(Some(sub_progress)), &mut out, &mut stderr()); - #[cfg(feature = "tracing")] - drop(span); + let res = gix::trace::coarse!("run", name = name).into_scope(|| { + run(progress::DoOrDiscard::from(Some(sub_progress)), &mut out, &mut stderr()) + }); tx.send(Event::ComputationDone(res, out)).ok(); } }); From 83895f964c8308902d083f2fd98a457926db24b3 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 10:22:34 +0200 Subject: [PATCH 4/8] feat: `Span::record()` to allow recording a value after the span was created. --- gix-trace/src/disabled.rs | 7 +++++++ gix-trace/src/enabled.rs | 36 +++++++++++++++++++++++++++++++----- gix-trace/src/lib.rs | 4 ++-- gix-trace/tests/trace.rs | 3 ++- 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/gix-trace/src/disabled.rs b/gix-trace/src/disabled.rs index f07a5bdeef5..3f6946f2e5a 100644 --- a/gix-trace/src/disabled.rs +++ b/gix-trace/src/disabled.rs @@ -3,6 +3,13 @@ #[derive(Clone)] pub struct Span; +impl Span { + /// A no-op + pub fn record(&self, _field: &str, _value: V) -> &Self { + self + } +} + /// A macro to create a span. #[macro_export] macro_rules! span { diff --git a/gix-trace/src/enabled.rs b/gix-trace/src/enabled.rs index 6dab16e6289..92f559a9c12 100644 --- a/gix-trace/src/enabled.rs +++ b/gix-trace/src/enabled.rs @@ -1,4 +1,4 @@ -use tracing_core::{dispatcher::get_default as with_dispatcher, span::Id, Dispatch}; +use tracing_core::{dispatcher::get_default as with_dispatcher, span, span::Id, Dispatch}; // these are used later in macros. pub use tracing_core::{field, metadata, Metadata}; @@ -6,11 +6,13 @@ pub use tracing_core::{field, metadata, Metadata}; /// An entered span which will exit on drop. #[derive(Clone)] pub struct Span { - id: Option<(Id, Dispatch)>, + id: Option<(Id, Dispatch, &'static Metadata<'static>)>, } impl Span { - #[allow(missing_docs)] + /// Create a new span. + /// + /// This constructor is typically invoked by the [`crate::span!`] macro. pub fn new( level: crate::Level, meta: &'static Metadata<'static>, @@ -23,16 +25,40 @@ impl Span { let id = dispatch.new_span(&tracing_core::span::Attributes::new(meta, values)); dispatch.enter(&id); Self { - id: Some((id, dispatch.clone())), + id: Some((id, dispatch.clone(), meta)), } }) } } + + /// Record a single `field` to take `value`. + /// + /// Note that this silently fails if the field name wasn't mentioned when the span was created. + pub fn record(&self, field: &str, value: V) -> &Self + where + V: field::Value, + { + if let Some((_, _, meta)) = &self.id { + let fields = meta.fields(); + if let Some(field) = fields.field(field) { + self.record_all(&fields.value_set(&[(&field, Some(&value as &dyn field::Value))])); + } + } + self + } + + fn record_all(&self, values: &field::ValueSet<'_>) -> &Self { + if let Some((id, dispatch, _)) = &self.id { + let record = span::Record::new(values); + dispatch.record(id, &record); + } + self + } } impl Drop for Span { fn drop(&mut self) { - if let Some((id, dispatch)) = self.id.take() { + if let Some((id, dispatch, _meta)) = self.id.take() { dispatch.exit(&id); dispatch.try_close(id); } diff --git a/gix-trace/src/lib.rs b/gix-trace/src/lib.rs index 5ba216efaa6..c156a4fd6e5 100644 --- a/gix-trace/src/lib.rs +++ b/gix-trace/src/lib.rs @@ -35,7 +35,7 @@ pub const MAX_LEVEL: Level = Level::Coarse; mod enabled; #[cfg(feature = "tracing")] -pub use enabled::Span; +pub use enabled::{field, Span}; impl Span { /// Execute `f` in with this span active, consuming it. @@ -46,7 +46,7 @@ impl Span { #[cfg(feature = "tracing")] #[doc(hidden)] -pub use enabled::{field, metadata, MetaOnlyCallsite, Metadata}; +pub use enabled::{metadata, MetaOnlyCallsite, Metadata}; #[cfg(not(feature = "tracing"))] mod disabled; diff --git a/gix-trace/tests/trace.rs b/gix-trace/tests/trace.rs index 5ecdab54c68..219729cecc9 100644 --- a/gix-trace/tests/trace.rs +++ b/gix-trace/tests/trace.rs @@ -4,7 +4,8 @@ fn span() { let _x = span!(gix_trace::Level::Coarse, "hello"); let fourty_two = span!(gix_trace::Level::Coarse, "hello", x = "value", y = 42).into_scope(|| 42); assert_eq!(fourty_two, 42); - span!(target: "other", gix_trace::Level::Coarse, "hello", x = "value", y = 42); + let span = span!(target: "other", gix_trace::Level::Coarse, "hello", x = "value", y = 42); + span.record("y", "hello").record("x", 36); } #[test] From 8a4a0af871c9a666518c65e8de683c7aec025c3b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 11:01:51 +0200 Subject: [PATCH 5/8] refactor - split corpus modules out into files --- gitoxide-core/src/corpus/mod.rs | 99 +------------------------------ gitoxide-core/src/corpus/run.rs | 55 +++++++++++++++++ gitoxide-core/src/corpus/trace.rs | 38 ++++++++++++ 3 files changed, 95 insertions(+), 97 deletions(-) create mode 100644 gitoxide-core/src/corpus/run.rs create mode 100644 gitoxide-core/src/corpus/trace.rs diff --git a/gitoxide-core/src/corpus/mod.rs b/gitoxide-core/src/corpus/mod.rs index c6608a272b5..589a574d3a7 100644 --- a/gitoxide-core/src/corpus/mod.rs +++ b/gitoxide-core/src/corpus/mod.rs @@ -38,100 +38,5 @@ pub(crate) struct Run { error: Option, } -pub(crate) mod trace { - use rusqlite::params; - use std::path::Path; - use std::sync::atomic::{AtomicU32, Ordering}; - use std::sync::{Arc, Mutex}; - use tracing_forest::tree::Tree; - use tracing_subscriber::layer::SubscriberExt; - - pub fn override_thread_subscriber( - db_path: impl AsRef, - ) -> anyhow::Result<(tracing::subscriber::DefaultGuard, Arc)> { - let current_id = Arc::new(AtomicU32::default()); - let processor = tracing_forest::Printer::new().formatter(StoreTreeToDb { - con: Arc::new(Mutex::new(rusqlite::Connection::open(&db_path)?)), - run_id: current_id.clone(), - }); - let subscriber = tracing_subscriber::Registry::default().with(tracing_forest::ForestLayer::from(processor)); - let guard = tracing::subscriber::set_default(subscriber); - Ok((guard, current_id)) - } - - pub struct StoreTreeToDb { - pub con: Arc>, - pub run_id: Arc, - } - impl tracing_forest::printer::Formatter for StoreTreeToDb { - type Error = rusqlite::Error; - - fn fmt(&self, tree: &Tree) -> Result { - let json = serde_json::to_string_pretty(&tree).expect("serialization to string always works"); - let run_id = self.run_id.load(Ordering::SeqCst); - self.con - .lock() - .unwrap() - .execute("UPDATE run SET spans_json = ?1 WHERE id = ?2", params![json, run_id])?; - Ok(String::new()) - } - } -} - -pub(crate) mod run { - use crate::corpus; - use crate::corpus::{Run, Task}; - use std::path::Path; - use std::sync::atomic::AtomicBool; - - impl Task { - pub fn perform( - &self, - run: &mut Run, - repo: &Path, - progress: &mut corpus::Progress, - threads: Option, - should_interrupt: &AtomicBool, - ) { - let start = std::time::Instant::now(); - if let Err(err) = self.execute.execute(repo, progress, threads, should_interrupt) { - run.error = Some(format!("{err:#?}")) - } - run.duration = start.elapsed(); - } - } - - /// Note that once runs have been recorded, the implementation must not change anymore to keep it comparable. - /// If changes have be done, rather change the name of the owning task to start a new kind of task. - pub(crate) trait Execute { - fn execute( - &self, - repo: &Path, - progress: &mut corpus::Progress, - threads: Option, - should_interrupt: &AtomicBool, - ) -> anyhow::Result<()>; - } - - pub(crate) static ALL: &[Task] = &[Task { - short_name: "OPNR", - description: "open repository (isolated)", - execute_exclusive: false, - execute: &OpenRepo, - }]; - - struct OpenRepo; - - impl Execute for OpenRepo { - fn execute( - &self, - repo: &Path, - _progress: &mut corpus::Progress, - _threads: Option, - _should_interrupt: &AtomicBool, - ) -> anyhow::Result<()> { - gix::open_opts(repo, gix::open::Options::isolated())?; - Ok(()) - } - } -} +pub(crate) mod run; +pub(crate) mod trace; diff --git a/gitoxide-core/src/corpus/run.rs b/gitoxide-core/src/corpus/run.rs new file mode 100644 index 00000000000..28df0453f84 --- /dev/null +++ b/gitoxide-core/src/corpus/run.rs @@ -0,0 +1,55 @@ +use crate::corpus; +use crate::corpus::{Run, Task}; +use std::path::Path; +use std::sync::atomic::AtomicBool; + +impl Task { + pub fn perform( + &self, + run: &mut Run, + repo: &Path, + progress: &mut corpus::Progress, + threads: Option, + should_interrupt: &AtomicBool, + ) { + let start = std::time::Instant::now(); + if let Err(err) = self.execute.execute(repo, progress, threads, should_interrupt) { + run.error = Some(format!("{err:#?}")) + } + run.duration = start.elapsed(); + } +} + +/// Note that once runs have been recorded, the implementation must not change anymore to keep it comparable. +/// If changes have be done, rather change the name of the owning task to start a new kind of task. +pub(crate) trait Execute { + fn execute( + &self, + repo: &Path, + progress: &mut corpus::Progress, + threads: Option, + should_interrupt: &AtomicBool, + ) -> anyhow::Result<()>; +} + +pub(crate) static ALL: &[Task] = &[Task { + short_name: "OPNR", + description: "open repository (isolated)", + execute_exclusive: false, + execute: &OpenRepo, +}]; + +struct OpenRepo; + +impl Execute for OpenRepo { + fn execute( + &self, + repo: &Path, + _progress: &mut corpus::Progress, + _threads: Option, + _should_interrupt: &AtomicBool, + ) -> anyhow::Result<()> { + gix::open_opts(repo, gix::open::Options::isolated())?; + Ok(()) + } +} diff --git a/gitoxide-core/src/corpus/trace.rs b/gitoxide-core/src/corpus/trace.rs new file mode 100644 index 00000000000..8a7be071636 --- /dev/null +++ b/gitoxide-core/src/corpus/trace.rs @@ -0,0 +1,38 @@ +use rusqlite::params; +use std::path::Path; +use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::{Arc, Mutex}; +use tracing_forest::tree::Tree; +use tracing_subscriber::layer::SubscriberExt; + +pub fn override_thread_subscriber( + db_path: impl AsRef, +) -> anyhow::Result<(tracing::subscriber::DefaultGuard, Arc)> { + let current_id = Arc::new(AtomicU32::default()); + let processor = tracing_forest::Printer::new().formatter(StoreTreeToDb { + con: Arc::new(Mutex::new(rusqlite::Connection::open(&db_path)?)), + run_id: current_id.clone(), + }); + let subscriber = tracing_subscriber::Registry::default().with(tracing_forest::ForestLayer::from(processor)); + let guard = tracing::subscriber::set_default(subscriber); + Ok((guard, current_id)) +} + +pub struct StoreTreeToDb { + pub con: Arc>, + pub run_id: Arc, +} + +impl tracing_forest::printer::Formatter for StoreTreeToDb { + type Error = rusqlite::Error; + + fn fmt(&self, tree: &Tree) -> Result { + let json = serde_json::to_string_pretty(&tree).expect("serialization to string always works"); + let run_id = self.run_id.load(Ordering::SeqCst); + self.con + .lock() + .unwrap() + .execute("UPDATE run SET spans_json = ?1 WHERE id = ?2", params![json, run_id])?; + Ok(String::new()) + } +} From 0f973ac53df631ad2abdf85dbe2453e528c7e6c3 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 11:02:28 +0200 Subject: [PATCH 6/8] gix-corpus now respects the --trace flag --- Cargo.lock | 1 + gitoxide-core/Cargo.toml | 3 ++- gitoxide-core/src/corpus/engine.rs | 24 ++++++++++++++---- gitoxide-core/src/corpus/mod.rs | 2 ++ gitoxide-core/src/corpus/trace.rs | 35 ++++++++++++++++++++++++--- src/plumbing/main.rs | 39 ++++++++++++++++++------------ src/shared.rs | 2 +- 7 files changed, 80 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e23021c2232..97b4c4caafc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1268,6 +1268,7 @@ dependencies = [ "jwalk", "layout-rs", "open", + "parking_lot", "rusqlite", "serde", "serde_json", diff --git a/gitoxide-core/Cargo.toml b/gitoxide-core/Cargo.toml index b814f130a42..64eb4b707ac 100644 --- a/gitoxide-core/Cargo.toml +++ b/gitoxide-core/Cargo.toml @@ -23,7 +23,7 @@ estimate-hours = ["dep:itertools", "dep:fs-err", "dep:crossbeam-channel", "dep:s query = ["dep:rusqlite"] ## Run algorithms on a corpus of repositories and store their results for later comparison and intelligence gathering. ## *Note that* `organize` we need for finding git repositories fast. -corpus = [ "dep:rusqlite", "dep:sysinfo", "organize", "dep:crossbeam-channel", "dep:serde_json", "dep:tracing-forest", "dep:tracing-subscriber", "dep:tracing" ] +corpus = [ "dep:rusqlite", "dep:sysinfo", "organize", "dep:crossbeam-channel", "dep:serde_json", "dep:tracing-forest", "dep:tracing-subscriber", "dep:tracing", "dep:parking_lot" ] #! ### Mutually Exclusive Networking #! If both are set, _blocking-client_ will take precedence, allowing `--all-features` to be used. @@ -72,6 +72,7 @@ smallvec = { version = "1.10.0", optional = true } rusqlite = { version = "0.29.0", optional = true, features = ["bundled"] } # for 'corpus' +parking_lot = { version = "0.12.1", optional = true } sysinfo = { version = "0.29.2", optional = true, default-features = false } serde_json = { version = "1.0.65", optional = true } tracing-forest = { version = "0.1.5", features = ["serde"], optional = true } diff --git a/gitoxide-core/src/corpus/engine.rs b/gitoxide-core/src/corpus/engine.rs index 85c43236b02..bf4bfc87844 100644 --- a/gitoxide-core/src/corpus/engine.rs +++ b/gitoxide-core/src/corpus/engine.rs @@ -12,12 +12,20 @@ use std::time::{Duration, Instant}; impl Engine { /// Open the corpus DB or create it. - pub fn open_or_create(db: PathBuf, gitoxide_version: String, progress: corpus::Progress) -> anyhow::Result { + pub fn open_or_create( + db: PathBuf, + gitoxide_version: String, + progress: corpus::Progress, + trace_to_progress: bool, + reverse_trace_lines: bool, + ) -> anyhow::Result { let con = crate::corpus::db::create(db).context("Could not open or create database")?; Ok(Engine { progress, con, gitoxide_version, + trace_to_progress, + reverse_trace_lines, }) } @@ -66,7 +74,11 @@ impl Engine { if task.execute_exclusive || threads == 1 { let mut run_progress = repo_progress.add_child("set later"); - let (_guard, current_id) = corpus::trace::override_thread_subscriber(db_path.as_str())?; + let (_guard, current_id) = corpus::trace::override_thread_subscriber( + db_path.as_str(), + self.trace_to_progress.then(|| task_progress.add_child("trace")), + self.reverse_trace_lines, + )?; for repo in &repos { if gix::interrupt::is_triggered() { @@ -80,7 +92,7 @@ impl Engine { .display() )); - // TODO: wait for new release to be able to provide run_id via span attributes + // TODO: wait for new release of `tracing-forest` to be able to provide run_id via span attributes let mut run = Self::insert_run(&self.con, gitoxide_id, runner_id, *task_id, repo.id)?; current_id.store(run.id, Ordering::SeqCst); tracing::info_span!("run", run_id = run.id).in_scope(|| { @@ -106,9 +118,11 @@ impl Engine { let shared_repo_progress = repo_progress.clone(); let db_path = db_path.clone(); move |tid| { + let mut progress = gix::threading::lock(&shared_repo_progress); ( - corpus::trace::override_thread_subscriber(db_path.as_str()), - gix::threading::lock(&shared_repo_progress).add_child(format!("{tid}")), + // threaded printing is usually spammy, and lines interleave so it's useless. + corpus::trace::override_thread_subscriber(db_path.as_str(), None, false), + progress.add_child(format!("{tid}")), rusqlite::Connection::open(&db_path), ) } diff --git a/gitoxide-core/src/corpus/mod.rs b/gitoxide-core/src/corpus/mod.rs index 589a574d3a7..8e49b9a05c3 100644 --- a/gitoxide-core/src/corpus/mod.rs +++ b/gitoxide-core/src/corpus/mod.rs @@ -5,6 +5,8 @@ pub struct Engine { progress: Progress, con: rusqlite::Connection, gitoxide_version: String, + trace_to_progress: bool, + reverse_trace_lines: bool, } pub struct RunOutcome { diff --git a/gitoxide-core/src/corpus/trace.rs b/gitoxide-core/src/corpus/trace.rs index 8a7be071636..614d4b27f0c 100644 --- a/gitoxide-core/src/corpus/trace.rs +++ b/gitoxide-core/src/corpus/trace.rs @@ -1,17 +1,25 @@ +use gix::progress::DoOrDiscard; +use parking_lot::Mutex; use rusqlite::params; use std::path::Path; use std::sync::atomic::{AtomicU32, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use tracing_forest::tree::Tree; use tracing_subscriber::layer::SubscriberExt; +type ProgressItem = DoOrDiscard; + pub fn override_thread_subscriber( db_path: impl AsRef, + progress: Option, + reverse_lines: bool, ) -> anyhow::Result<(tracing::subscriber::DefaultGuard, Arc)> { let current_id = Arc::new(AtomicU32::default()); let processor = tracing_forest::Printer::new().formatter(StoreTreeToDb { con: Arc::new(Mutex::new(rusqlite::Connection::open(&db_path)?)), run_id: current_id.clone(), + progress: progress.map(Mutex::new), + reverse_lines, }); let subscriber = tracing_subscriber::Registry::default().with(tracing_forest::ForestLayer::from(processor)); let guard = tracing::subscriber::set_default(subscriber); @@ -19,19 +27,38 @@ pub fn override_thread_subscriber( } pub struct StoreTreeToDb { - pub con: Arc>, - pub run_id: Arc, + con: Arc>, + run_id: Arc, + progress: Option>, + reverse_lines: bool, } impl tracing_forest::printer::Formatter for StoreTreeToDb { type Error = rusqlite::Error; fn fmt(&self, tree: &Tree) -> Result { + if let Some((progress, tree)) = self + .progress + .as_ref() + .map(Mutex::lock) + .zip(tracing_forest::printer::Pretty.fmt(tree).ok()) + { + use gix::Progress; + if self.reverse_lines { + for line in tree.lines().rev() { + progress.info(line); + } + } else { + for line in tree.lines() { + progress.info(line); + } + } + } + // TODO: wait for new release of `tracing-forest` and load the ID from span fields. let json = serde_json::to_string_pretty(&tree).expect("serialization to string always works"); let run_id = self.run_id.load(Ordering::SeqCst); self.con .lock() - .unwrap() .execute("UPDATE run SET spans_json = ?1 WHERE id = ?2", params![json, run_id])?; Ok(String::new()) } diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 306b9eae51a..bdcb9013f64 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -136,21 +136,30 @@ pub fn main() -> Result<()> { match cmd { #[cfg(feature = "gitoxide-core-tools-corpus")] - Subcommands::Corpus(crate::plumbing::options::corpus::Platform { db, path, cmd }) => prepare_and_run( - "corpus", - trace, - auto_verbose, - progress, - progress_keep_open, - core::corpus::PROGRESS_RANGE, - move |progress, _out, _err| { - let mut engine = core::corpus::Engine::open_or_create(db, env!("GITOXIDE_VERSION").into(), progress)?; - match cmd { - crate::plumbing::options::corpus::SubCommands::Run => engine.run(path, thread_limit), - crate::plumbing::options::corpus::SubCommands::Refresh => engine.refresh(path), - } - }, - ), + Subcommands::Corpus(crate::plumbing::options::corpus::Platform { db, path, cmd }) => { + let reverse_trace_lines = progress; + prepare_and_run( + "corpus", + trace, + auto_verbose, + progress, + progress_keep_open, + core::corpus::PROGRESS_RANGE, + move |progress, _out, _err| { + let mut engine = core::corpus::Engine::open_or_create( + db, + env!("GITOXIDE_VERSION").into(), + progress, + trace, + reverse_trace_lines, + )?; + match cmd { + crate::plumbing::options::corpus::SubCommands::Run => engine.run(path, thread_limit), + crate::plumbing::options::corpus::SubCommands::Refresh => engine.refresh(path), + } + }, + ) + } Subcommands::CommitGraph(cmd) => match cmd { commitgraph::Subcommands::List { spec } => prepare_and_run( "commitgraph-list", diff --git a/src/shared.rs b/src/shared.rs index 8b5e640a4fe..9b775d9b66d 100644 --- a/src/shared.rs +++ b/src/shared.rs @@ -108,8 +108,8 @@ pub mod pretty { move |tree: &tracing_forest::tree::Tree| -> Result { use gix::Progress; use tracing_forest::Formatter; - let tree = tracing_forest::printer::Pretty.fmt(tree)?; let progress = &mut progress.lock().unwrap(); + let tree = tracing_forest::printer::Pretty.fmt(tree)?; if reverse_lines { for line in tree.lines().rev() { progress.info(line); From 4cef57db735c20da3fa1c56d9c1744e4f653bce0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 14:36:58 +0200 Subject: [PATCH 7/8] Add `corpus --dry-run` and `--task-sql-suffix` and `--repo-sql-suffix` --- gitoxide-core/src/corpus/db.rs | 19 ++++- gitoxide-core/src/corpus/engine.rs | 120 ++++++++++++++++++++--------- gitoxide-core/src/corpus/mod.rs | 8 +- gitoxide-core/src/corpus/run.rs | 6 +- src/plumbing/main.rs | 16 ++-- src/plumbing/options/mod.rs | 18 ++++- 6 files changed, 131 insertions(+), 56 deletions(-) diff --git a/gitoxide-core/src/corpus/db.rs b/gitoxide-core/src/corpus/db.rs index b861b070e24..317cc1c71e0 100644 --- a/gitoxide-core/src/corpus/db.rs +++ b/gitoxide-core/src/corpus/db.rs @@ -173,12 +173,25 @@ impl Engine { .con .query_row( "INSERT INTO gitoxide_version (version) VALUES (?1) ON CONFLICT DO UPDATE SET version = version RETURNING id", - [&self.gitoxide_version], + [&self.state.gitoxide_version], |r| r.get(0), )?) } - pub(crate) fn tasks_or_insert(&self) -> anyhow::Result> { - let mut out: Vec<_> = super::run::ALL.iter().map(|task| (0, task)).collect(); + pub(crate) fn tasks_or_insert( + &self, + allowed_short_names: &[String], + ) -> anyhow::Result> { + let mut out: Vec<_> = super::run::ALL + .iter() + .filter(|task| { + if allowed_short_names.is_empty() { + true + } else { + allowed_short_names.iter().any(|allowed| task.short_name == allowed) + } + }) + .map(|task| (0, task)) + .collect(); for (id, task) in &mut out { *id = self.con.query_row( "INSERT INTO task (short_name, description) VALUES (?1, ?2) ON CONFLICT DO UPDATE SET short_name = short_name, description = ?2 RETURNING id", diff --git a/gitoxide-core/src/corpus/engine.rs b/gitoxide-core/src/corpus/engine.rs index bf4bfc87844..91155107c1f 100644 --- a/gitoxide-core/src/corpus/engine.rs +++ b/gitoxide-core/src/corpus/engine.rs @@ -10,40 +10,47 @@ use std::path::{Path, PathBuf}; use std::sync::atomic::Ordering; use std::time::{Duration, Instant}; +pub type ProgressItem = gix::progress::DoOrDiscard; + +pub struct State { + pub progress: ProgressItem, + pub gitoxide_version: String, + pub trace_to_progress: bool, + pub reverse_trace_lines: bool, +} + impl Engine { /// Open the corpus DB or create it. - pub fn open_or_create( - db: PathBuf, - gitoxide_version: String, - progress: corpus::Progress, - trace_to_progress: bool, - reverse_trace_lines: bool, - ) -> anyhow::Result { + pub fn open_or_create(db: PathBuf, state: State) -> anyhow::Result { let con = crate::corpus::db::create(db).context("Could not open or create database")?; - Ok(Engine { - progress, - con, - gitoxide_version, - trace_to_progress, - reverse_trace_lines, - }) + Ok(Engine { con, state }) } /// Run on the existing set of repositories we have already seen or obtain them from `path` if there is none yet. - pub fn run(&mut self, corpus_path: PathBuf, threads: Option) -> anyhow::Result<()> { + pub fn run( + &mut self, + corpus_path: PathBuf, + threads: Option, + dry_run: bool, + repo_sql_suffix: Option, + allowed_task_names: Vec, + ) -> anyhow::Result<()> { + let tasks = self.tasks_or_insert(&allowed_task_names)?; + if tasks.is_empty() { + bail!("Cannot run without any task to perform on the repositories"); + } let (corpus_path, corpus_id) = self.prepare_corpus_path(corpus_path)?; let gitoxide_id = self.gitoxide_version_id_or_insert()?; let runner_id = self.runner_id_or_insert()?; - let repos = self.find_repos_or_insert(&corpus_path, corpus_id)?; - let tasks = self.tasks_or_insert()?; - self.perform_run(&corpus_path, gitoxide_id, runner_id, &tasks, repos, threads) + let repos = self.find_repos_or_insert(&corpus_path, corpus_id, repo_sql_suffix)?; + self.perform_run(&corpus_path, gitoxide_id, runner_id, &tasks, repos, threads, dry_run) } pub fn refresh(&mut self, corpus_path: PathBuf) -> anyhow::Result<()> { let (corpus_path, corpus_id) = self.prepare_corpus_path(corpus_path)?; let repos = self.refresh_repos(&corpus_path, corpus_id)?; - self.progress.set_name("refresh repos"); - self.progress.info(format!( + self.state.progress.set_name("refresh repos"); + self.state.progress.info(format!( "Added or updated {} repositories under {corpus_path:?}", repos.len() )); @@ -52,6 +59,7 @@ impl Engine { } impl Engine { + #[allow(clippy::too_many_arguments)] fn perform_run( &mut self, corpus_path: &Path, @@ -60,9 +68,10 @@ impl Engine { tasks: &[(db::Id, &'static Task)], mut repos: Vec, threads: Option, + dry_run: bool, ) -> anyhow::Result<()> { let start = Instant::now(); - let task_progress = &mut self.progress; + let task_progress = &mut self.state.progress; task_progress.set_name("run"); task_progress.init(Some(tasks.len()), gix::progress::count("tasks")); let threads = gix::parallel::num_threads(threads); @@ -70,14 +79,31 @@ impl Engine { for (task_id, task) in tasks { let task_start = Instant::now(); let mut repo_progress = task_progress.add_child(format!("run '{}'", task.short_name)); - repo_progress.init(Some(repos.len()), gix::progress::count("repos")); - - if task.execute_exclusive || threads == 1 { + if task.execute_exclusive || threads == 1 || dry_run { + if dry_run { + task_progress.set_name("WOULD run"); + for repo in &repos { + task_progress.info(format!( + "{}", + repo.path + .strip_prefix(corpus_path) + .expect("corpus contains repo") + .display() + )); + task_progress.inc(); + } + task_progress.info(format!("with {} tasks", tasks.len())); + for (_, task) in tasks { + task_progress.info(format!("task '{}' ({})", task.description, task.short_name)) + } + continue; + } + repo_progress.init(Some(repos.len()), gix::progress::count("repos")); let mut run_progress = repo_progress.add_child("set later"); let (_guard, current_id) = corpus::trace::override_thread_subscriber( db_path.as_str(), - self.trace_to_progress.then(|| task_progress.add_child("trace")), - self.reverse_trace_lines, + self.state.trace_to_progress.then(|| task_progress.add_child("trace")), + self.state.reverse_trace_lines, )?; for repo in &repos { @@ -180,13 +206,21 @@ impl Engine { Ok((corpus_path, corpus_id)) } - fn find_repos(&mut self, corpus_path: &Path, corpus_id: db::Id) -> anyhow::Result> { - self.progress.set_name("query db-repos"); - self.progress.init(None, gix::progress::count("repos")); + fn find_repos( + &mut self, + corpus_path: &Path, + corpus_id: db::Id, + sql_suffix: Option<&str>, + ) -> anyhow::Result> { + self.state.progress.set_name("query db-repos"); + self.state.progress.init(None, gix::progress::count("repos")); Ok(self .con - .prepare("SELECT id, rela_path, odb_size, num_objects, num_references FROM repository WHERE corpus = ?1")? + .prepare(&format!( + "SELECT id, rela_path, odb_size, num_objects, num_references FROM repository WHERE corpus = ?1 {}", + sql_suffix.unwrap_or_default() + ))? .query_map([corpus_id], |r| { Ok(db::Repo { id: r.get(0)?, @@ -196,17 +230,17 @@ impl Engine { num_references: r.get(4)?, }) })? - .inspect(|_| self.progress.inc()) + .inspect(|_| self.state.progress.inc()) .collect::>()?) } fn refresh_repos(&mut self, corpus_path: &Path, corpus_id: db::Id) -> anyhow::Result> { let start = Instant::now(); - self.progress.set_name("refresh"); - self.progress.init(None, gix::progress::count("repos")); + self.state.progress.set_name("refresh"); + self.state.progress.init(None, gix::progress::count("repos")); let repos = std::thread::scope({ - let progress = &mut self.progress; + let progress = &mut self.state.progress; let con = &mut self.con; |scope| -> anyhow::Result<_> { let threads = std::thread::available_parallelism() @@ -278,13 +312,23 @@ impl Engine { Ok(repos) } - fn find_repos_or_insert(&mut self, corpus_path: &Path, corpus_id: db::Id) -> anyhow::Result> { + fn find_repos_or_insert( + &mut self, + corpus_path: &Path, + corpus_id: db::Id, + sql_suffix: Option, + ) -> anyhow::Result> { let start = Instant::now(); - let repos = self.find_repos(corpus_path, corpus_id)?; + let repos = self.find_repos(corpus_path, corpus_id, sql_suffix.as_deref())?; if repos.is_empty() { - self.refresh_repos(corpus_path, corpus_id) + let res = self.refresh_repos(corpus_path, corpus_id); + if sql_suffix.is_some() { + self.find_repos(corpus_path, corpus_id, sql_suffix.as_deref()) + } else { + res + } } else { - self.progress.show_throughput(start); + self.state.progress.show_throughput(start); Ok(repos) } } diff --git a/gitoxide-core/src/corpus/mod.rs b/gitoxide-core/src/corpus/mod.rs index 8e49b9a05c3..191352be8ec 100644 --- a/gitoxide-core/src/corpus/mod.rs +++ b/gitoxide-core/src/corpus/mod.rs @@ -1,12 +1,8 @@ pub const PROGRESS_RANGE: std::ops::RangeInclusive = 0..=3; -pub(crate) type Progress = gix::progress::DoOrDiscard; pub struct Engine { - progress: Progress, con: rusqlite::Connection, - gitoxide_version: String, - trace_to_progress: bool, - reverse_trace_lines: bool, + state: engine::State, } pub struct RunOutcome { @@ -15,7 +11,7 @@ pub struct RunOutcome { } pub(crate) mod db; -pub(crate) mod engine; +pub mod engine; /// Contains all information necessary to run a task. pub(crate) struct Task { diff --git a/gitoxide-core/src/corpus/run.rs b/gitoxide-core/src/corpus/run.rs index 28df0453f84..632e9e7f241 100644 --- a/gitoxide-core/src/corpus/run.rs +++ b/gitoxide-core/src/corpus/run.rs @@ -8,7 +8,7 @@ impl Task { &self, run: &mut Run, repo: &Path, - progress: &mut corpus::Progress, + progress: &mut corpus::engine::ProgressItem, threads: Option, should_interrupt: &AtomicBool, ) { @@ -26,7 +26,7 @@ pub(crate) trait Execute { fn execute( &self, repo: &Path, - progress: &mut corpus::Progress, + progress: &mut corpus::engine::ProgressItem, threads: Option, should_interrupt: &AtomicBool, ) -> anyhow::Result<()>; @@ -45,7 +45,7 @@ impl Execute for OpenRepo { fn execute( &self, repo: &Path, - _progress: &mut corpus::Progress, + _progress: &mut corpus::engine::ProgressItem, _threads: Option, _should_interrupt: &AtomicBool, ) -> anyhow::Result<()> { diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index bdcb9013f64..2c0bb6b1920 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -148,13 +148,19 @@ pub fn main() -> Result<()> { move |progress, _out, _err| { let mut engine = core::corpus::Engine::open_or_create( db, - env!("GITOXIDE_VERSION").into(), - progress, - trace, - reverse_trace_lines, + core::corpus::engine::State { + gitoxide_version: env!("GITOXIDE_VERSION").into(), + progress, + trace_to_progress: trace, + reverse_trace_lines, + }, )?; match cmd { - crate::plumbing::options::corpus::SubCommands::Run => engine.run(path, thread_limit), + crate::plumbing::options::corpus::SubCommands::Run { + dry_run, + repo_sql_suffix, + include_task, + } => engine.run(path, thread_limit, dry_run, repo_sql_suffix, include_task), crate::plumbing::options::corpus::SubCommands::Refresh => engine.refresh(path), } }, diff --git a/src/plumbing/options/mod.rs b/src/plumbing/options/mod.rs index db4f6306d26..82698bdc7b7 100644 --- a/src/plumbing/options/mod.rs +++ b/src/plumbing/options/mod.rs @@ -149,7 +149,23 @@ pub mod corpus { #[derive(Debug, clap::Subcommand)] pub enum SubCommands { /// Perform a corpus run on all registered repositories. - Run, + Run { + /// Don't run any task, but print all repos that would be traversed once. + /// + /// Note that this will refresh repositories if necessary and store them in the database, it just won't run tasks. + #[clap(long, short = 'n')] + dry_run: bool, + + /// The SQL that will be appended to the actual select statement for repositories to apply additional filtering, like `LIMIT 10`. + /// + /// The string must be trusted even though the engine will only execute a single statement. + #[clap(long, short = 'r')] + repo_sql_suffix: Option, + + /// The short_names of the tasks to include when running. + #[clap(long, short = 't')] + include_task: Vec, + }, /// Re-read all repositories under the corpus directory, and add or update them. Refresh, } From 694e572e2fbaade1fbd09dd166b23025c8435787 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 17 Jun 2023 16:40:25 +0200 Subject: [PATCH 8/8] add more tasks to gather a little more information --- gitoxide-core/src/corpus/engine.rs | 4 +- gitoxide-core/src/corpus/run.rs | 70 ++++++- gix-index/src/extension/tree/verify.rs | 1 + gix-index/src/file/verify.rs | 1 + gix-index/src/verify.rs | 1 + gix-odb/src/store_impls/dynamic/verify.rs | 215 +++++++++++----------- 6 files changed, 181 insertions(+), 111 deletions(-) diff --git a/gitoxide-core/src/corpus/engine.rs b/gitoxide-core/src/corpus/engine.rs index 91155107c1f..c7725265366 100644 --- a/gitoxide-core/src/corpus/engine.rs +++ b/gitoxide-core/src/corpus/engine.rs @@ -76,7 +76,7 @@ impl Engine { task_progress.init(Some(tasks.len()), gix::progress::count("tasks")); let threads = gix::parallel::num_threads(threads); let db_path = self.con.path().expect("opened from path on disk").to_owned(); - for (task_id, task) in tasks { + 'tasks_loop: for (task_id, task) in tasks { let task_start = Instant::now(); let mut repo_progress = task_progress.add_child(format!("run '{}'", task.short_name)); if task.execute_exclusive || threads == 1 || dry_run { @@ -96,7 +96,7 @@ impl Engine { for (_, task) in tasks { task_progress.info(format!("task '{}' ({})", task.description, task.short_name)) } - continue; + break 'tasks_loop; } repo_progress.init(Some(repos.len()), gix::progress::count("repos")); let mut run_progress = repo_progress.add_child("set later"); diff --git a/gitoxide-core/src/corpus/run.rs b/gitoxide-core/src/corpus/run.rs index 632e9e7f241..f494500b0d1 100644 --- a/gitoxide-core/src/corpus/run.rs +++ b/gitoxide-core/src/corpus/run.rs @@ -1,5 +1,6 @@ use crate::corpus; use crate::corpus::{Run, Task}; +use crate::pack::verify::Algorithm; use std::path::Path; use std::sync::atomic::AtomicBool; @@ -32,12 +33,26 @@ pub(crate) trait Execute { ) -> anyhow::Result<()>; } -pub(crate) static ALL: &[Task] = &[Task { - short_name: "OPNR", - description: "open repository (isolated)", - execute_exclusive: false, - execute: &OpenRepo, -}]; +pub(crate) static ALL: &[Task] = &[ + Task { + short_name: "OPNR", + description: "open repository (isolated)", + execute_exclusive: false, + execute: &OpenRepo, + }, + Task { + short_name: "POCN", + description: "packed object count", + execute_exclusive: false, + execute: &CountPackedObjects, + }, + Task { + short_name: "VERI", + description: "verify object database", + execute_exclusive: true, + execute: &VerifyOdb, + }, +]; struct OpenRepo; @@ -53,3 +68,46 @@ impl Execute for OpenRepo { Ok(()) } } + +struct CountPackedObjects; + +impl Execute for CountPackedObjects { + fn execute( + &self, + repo: &Path, + _progress: &mut corpus::engine::ProgressItem, + _threads: Option, + _should_interrupt: &AtomicBool, + ) -> anyhow::Result<()> { + let repo = gix::open_opts(repo, gix::open::Options::isolated())?; + repo.objects.packed_object_count()?; + Ok(()) + } +} + +struct VerifyOdb; + +impl Execute for VerifyOdb { + fn execute( + &self, + repo: &Path, + progress: &mut corpus::engine::ProgressItem, + threads: Option, + should_interrupt: &AtomicBool, + ) -> anyhow::Result<()> { + let repo = gix::open_opts(repo, gix::open::Options::isolated())?; + crate::repository::verify::integrity( + repo, + std::io::sink(), + progress, + should_interrupt, + crate::repository::verify::Context { + output_statistics: None, + thread_limit: threads, + verify_mode: Default::default(), + algorithm: Algorithm::LessTime, + }, + )?; + Ok(()) + } +} diff --git a/gix-index/src/extension/tree/verify.rs b/gix-index/src/extension/tree/verify.rs index 6280cecf881..793f3132500 100644 --- a/gix-index/src/extension/tree/verify.rs +++ b/gix-index/src/extension/tree/verify.rs @@ -111,6 +111,7 @@ impl Tree { } Ok(entries.into()) } + let _span = gix_features::trace::coarse!("gix_index::extension::Tree::verify()"); if !self.name.is_empty() { return Err(Error::RootWithName { diff --git a/gix-index/src/file/verify.rs b/gix-index/src/file/verify.rs index 6743b37a7c1..a9680845c0e 100644 --- a/gix-index/src/file/verify.rs +++ b/gix-index/src/file/verify.rs @@ -23,6 +23,7 @@ pub use error::Error; impl File { /// Verify the integrity of the index to assure its consistency. pub fn verify_integrity(&self) -> Result<(), Error> { + let _span = gix_features::trace::coarse!("gix_index::File::verify_integrity()"); let checksum = self.checksum.ok_or(Error::NoChecksum)?; let num_bytes_to_hash = self.path.metadata()?.len() - checksum.as_bytes().len() as u64; let should_interrupt = AtomicBool::new(false); diff --git a/gix-index/src/verify.rs b/gix-index/src/verify.rs index ba7ec3872a4..7782cccbcba 100644 --- a/gix-index/src/verify.rs +++ b/gix-index/src/verify.rs @@ -42,6 +42,7 @@ pub mod extensions { impl State { /// Assure our entries are consistent. pub fn verify_entries(&self) -> Result<(), entries::Error> { + let _span = gix_features::trace::coarse!("gix_index::File::verify_entries()"); let mut previous = None::<&crate::Entry>; for (idx, entry) in self.entries.iter().enumerate() { if let Some(prev) = previous { diff --git a/gix-odb/src/store_impls/dynamic/verify.rs b/gix-odb/src/store_impls/dynamic/verify.rs index f8484e5d182..d8039cbebcc 100644 --- a/gix-odb/src/store_impls/dynamic/verify.rs +++ b/gix-odb/src/store_impls/dynamic/verify.rs @@ -122,6 +122,7 @@ impl super::Store { C: pack::cache::DecodeEntry, F: Fn() -> C + Send + Clone, { + let _span = gix_features::trace::coarse!("gix_odb:Store::verify_integrity()"); let mut index = self.index.load(); if !index.is_initialized() { self.consolidate_with_disk_state(true, false)?; @@ -144,118 +145,126 @@ impl super::Store { .map_or_else(Default::default, std::ffi::OsStr::to_string_lossy) ) }; - for slot_index in &index.slot_indices { - let slot = &self.files[*slot_index]; - if slot.generation.load(Ordering::SeqCst) != index.generation { - return Err(integrity::Error::NeedsRetryDueToChangeOnDisk); - } - let files = slot.files.load(); - let files = Option::as_ref(&files).ok_or(integrity::Error::NeedsRetryDueToChangeOnDisk)?; - - let start = Instant::now(); - let (mut child_progress, num_objects, index_path) = match files { - IndexAndPacks::Index(bundle) => { - let index; - let index = match bundle.index.loaded() { - Some(index) => index.deref(), - None => { - index = pack::index::File::at(bundle.index.path(), self.object_hash)?; - &index - } - }; - let pack; - let data = match bundle.data.loaded() { - Some(pack) => pack.deref(), - None => { - pack = pack::data::File::at(bundle.data.path(), self.object_hash)?; - &pack - } - }; - let outcome = index.verify_integrity( - Some(pack::index::verify::PackContext { - data, - options: options.clone(), - }), - progress.add_child_with_id( - "verify index", - integrity::ProgressId::VerifyIndex(Default::default()).into(), - ), - should_interrupt, - )?; - statistics.push(IndexStatistics { - path: bundle.index.path().to_owned(), - statistics: SingleOrMultiStatistics::Single( - outcome - .pack_traverse_statistics - .expect("pack provided so there are stats"), - ), - }); - (outcome.progress, index.num_objects(), index.path().to_owned()) + gix_features::trace::detail!("verify indices").into_scope(|| { + for slot_index in &index.slot_indices { + let slot = &self.files[*slot_index]; + if slot.generation.load(Ordering::SeqCst) != index.generation { + return Err(integrity::Error::NeedsRetryDueToChangeOnDisk); } - IndexAndPacks::MultiIndex(bundle) => { - let index; - let index = match bundle.multi_index.loaded() { - Some(index) => index.deref(), - None => { - index = pack::multi_index::File::at(bundle.multi_index.path())?; - &index - } - }; - let outcome = index.verify_integrity( - progress.add_child_with_id( - "verify multi-index", - integrity::ProgressId::VerifyMultiIndex(Default::default()).into(), - ), - should_interrupt, - options.clone(), - )?; + let files = slot.files.load(); + let files = Option::as_ref(&files).ok_or(integrity::Error::NeedsRetryDueToChangeOnDisk)?; - let index_dir = bundle.multi_index.path().parent().expect("file in a directory"); - statistics.push(IndexStatistics { - path: Default::default(), - statistics: SingleOrMultiStatistics::Multi( - outcome - .pack_traverse_statistics - .into_iter() - .zip(index.index_names()) - .map(|(statistics, index_name)| (index_dir.join(index_name), statistics)) - .collect(), - ), - }); - (outcome.progress, index.num_objects(), index.path().to_owned()) - } - }; + let start = Instant::now(); + let (mut child_progress, num_objects, index_path) = match files { + IndexAndPacks::Index(bundle) => { + let index; + let index = match bundle.index.loaded() { + Some(index) => index.deref(), + None => { + index = pack::index::File::at(bundle.index.path(), self.object_hash)?; + &index + } + }; + let pack; + let data = match bundle.data.loaded() { + Some(pack) => pack.deref(), + None => { + pack = pack::data::File::at(bundle.data.path(), self.object_hash)?; + &pack + } + }; + let outcome = index.verify_integrity( + Some(pack::index::verify::PackContext { + data, + options: options.clone(), + }), + progress.add_child_with_id( + "verify index", + integrity::ProgressId::VerifyIndex(Default::default()).into(), + ), + should_interrupt, + )?; + statistics.push(IndexStatistics { + path: bundle.index.path().to_owned(), + statistics: SingleOrMultiStatistics::Single( + outcome + .pack_traverse_statistics + .expect("pack provided so there are stats"), + ), + }); + (outcome.progress, index.num_objects(), index.path().to_owned()) + } + IndexAndPacks::MultiIndex(bundle) => { + let index; + let index = match bundle.multi_index.loaded() { + Some(index) => index.deref(), + None => { + index = pack::multi_index::File::at(bundle.multi_index.path())?; + &index + } + }; + let outcome = index.verify_integrity( + progress.add_child_with_id( + "verify multi-index", + integrity::ProgressId::VerifyMultiIndex(Default::default()).into(), + ), + should_interrupt, + options.clone(), + )?; - child_progress.set_name(index_check_message(&index_path)); - child_progress.show_throughput_with( - start, - num_objects as usize, - gix_features::progress::count("objects").expect("set"), - MessageLevel::Success, - ); - progress.inc(); - } + let index_dir = bundle.multi_index.path().parent().expect("file in a directory"); + statistics.push(IndexStatistics { + path: Default::default(), + statistics: SingleOrMultiStatistics::Multi( + outcome + .pack_traverse_statistics + .into_iter() + .zip(index.index_names()) + .map(|(statistics, index_name)| (index_dir.join(index_name), statistics)) + .collect(), + ), + }); + (outcome.progress, index.num_objects(), index.path().to_owned()) + } + }; + + child_progress.set_name(index_check_message(&index_path)); + child_progress.show_throughput_with( + start, + num_objects as usize, + gix_features::progress::count("objects").expect("set"), + MessageLevel::Success, + ); + progress.inc(); + } + Ok(()) + })?; progress.init( Some(index.loose_dbs.len()), gix_features::progress::count("loose object stores"), ); let mut loose_object_stores = Vec::new(); - for loose_db in &*index.loose_dbs { - let out = loose_db - .verify_integrity( - progress.add_child_with_id( - loose_db.path().display().to_string(), - integrity::ProgressId::VerifyLooseObjectDbPath.into(), - ), - should_interrupt, - ) - .map(|statistics| integrity::LooseObjectStatistics { - path: loose_db.path().to_owned(), - statistics, - })?; - loose_object_stores.push(out); - } + gix_features::trace::detail!("verify loose ODBs").into_scope( + || -> Result<_, crate::loose::verify::integrity::Error> { + for loose_db in &*index.loose_dbs { + let out = loose_db + .verify_integrity( + progress.add_child_with_id( + loose_db.path().display().to_string(), + integrity::ProgressId::VerifyLooseObjectDbPath.into(), + ), + should_interrupt, + ) + .map(|statistics| integrity::LooseObjectStatistics { + path: loose_db.path().to_owned(), + statistics, + })?; + loose_object_stores.push(out); + } + Ok(()) + }, + )?; Ok(integrity::Outcome { loose_object_stores,