Skip to content

Commit 5346ed2

Browse files
MegaRedHandedg-l
andauthored
refactor(l1): remove Mutex from profiling metrics (#5031)
**Motivation** Having a centralized `Mutex` in the profiling metrics could potentially make code slower, the more functions we instrument. Removing it reduces the noise we have in our measurements. **Description** This PR replaces the singleton `Mutex` by using the internal `RwLock` that spans already have for layers to store things. Disabling metrics doesn't seem to increase performance in a noticeable manner, so this shouldn't increase performance. --------- Co-authored-by: Edgar <[email protected]>
1 parent c638968 commit 5346ed2

File tree

2 files changed

+14
-13
lines changed

2 files changed

+14
-13
lines changed

cmd/ethrex/initializers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ pub fn init_tracing(opts: &Options) -> reload::Handle<EnvFilter, Registry> {
6767
let fmt_layer = layer.with_filter(filter);
6868

6969
let subscriber: Box<dyn tracing::Subscriber + Send + Sync> = if opts.metrics_enabled {
70-
let profiling_layer = FunctionProfilingLayer::default();
70+
let profiling_layer = FunctionProfilingLayer;
7171
Box::new(Registry::default().with(fmt_layer).with(profiling_layer))
7272
} else {
7373
Box::new(Registry::default().with(fmt_layer))

crates/blockchain/metrics/profiling.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
use prometheus::{Encoder, HistogramTimer, HistogramVec, TextEncoder, register_histogram_vec};
2-
use std::{
3-
collections::HashMap,
4-
sync::{LazyLock, Mutex},
5-
};
2+
use std::sync::LazyLock;
63
use tracing::{Subscriber, span::Id};
74
use tracing_subscriber::{Layer, layer::Context, registry::LookupSpan};
85

@@ -23,9 +20,10 @@ fn initialize_histogram_vec() -> HistogramVec {
2320
// We use this struct to simplify accumulating the time spent doing each task and publishing the metric only when the sync cycle is finished
2421
// We need to do this because things like database reads and writes are spread out throughout the code, so we need to gather multiple measurements to publish
2522
#[derive(Default)]
26-
pub struct FunctionProfilingLayer {
27-
function_timers: Mutex<HashMap<Id, HistogramTimer>>,
28-
}
23+
pub struct FunctionProfilingLayer;
24+
25+
/// Wrapper around [`HistogramTimer`] to avoid conflicts with other layers
26+
struct ProfileTimer(HistogramTimer);
2927

3028
impl<S> Layer<S> for FunctionProfilingLayer
3129
where
@@ -40,14 +38,17 @@ where
4038
let timer = METRICS_BLOCK_PROCESSING_PROFILE
4139
.with_label_values(&[name])
4240
.start_timer();
43-
let mut timers = self.function_timers.lock().unwrap();
44-
timers.insert(id.clone(), timer);
41+
// PERF: `extensions_mut` uses a Mutex internally (per span)
42+
span.extensions_mut().insert(ProfileTimer(timer));
4543
}
4644
}
4745

48-
fn on_exit(&self, id: &Id, _ctx: Context<'_, S>) {
49-
let mut timers = self.function_timers.lock().unwrap();
50-
if let Some(timer) = timers.remove(id) {
46+
fn on_exit(&self, id: &Id, ctx: Context<'_, S>) {
47+
let timer = ctx
48+
.span(id)
49+
// PERF: `extensions_mut` uses a Mutex internally (per span)
50+
.and_then(|span| span.extensions_mut().remove::<ProfileTimer>());
51+
if let Some(ProfileTimer(timer)) = timer {
5152
timer.observe_duration();
5253
}
5354
}

0 commit comments

Comments
 (0)