Skip to content

Commit 180d065

Browse files
patrick-ogradypatrick-ogrady
committed
cleanup
1 parent a1b684f commit 180d065

9 files changed

Lines changed: 35 additions & 83 deletions

File tree

‎consensus/src/aggregation/engine.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ impl<
337337
result,
338338
duration,
339339
} = request;
340-
duration.observe_now(self.context.as_ref());
340+
duration.record(self.context.as_ref());
341341
match result {
342342
Err(err) => {
343343
warn!(?err, %height, "automaton returned error");

‎consensus/src/marshal/coding/marshaled.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ where
448448
},
449449
is_valid = validity_request => is_valid,
450450
};
451-
verify_duration.observe_now(&runtime_context);
451+
verify_duration.record(&runtime_context);
452452
if application_valid {
453453
// The block is only persisted at this point.
454454
marshal.verified(round, block).await;
@@ -575,7 +575,7 @@ where
575575
result = parent_request => match result {
576576
Ok(parent) => parent,
577577
Err(_) => {
578-
parent_fetch_duration.observe_now(&runtime_context);
578+
parent_fetch_duration.record(&runtime_context);
579579
debug!(
580580
?parent_commitment,
581581
reason = "failed to fetch parent block",
@@ -585,7 +585,7 @@ where
585585
}
586586
},
587587
};
588-
parent_fetch_duration.observe_now(&runtime_context);
588+
parent_fetch_duration.record(&runtime_context);
589589

590590
// Special case: If the parent block is the last block in the epoch,
591591
// re-propose it as to not produce any blocks that will be cut out
@@ -628,7 +628,7 @@ where
628628
result = build_request => match result {
629629
Some(block) => block,
630630
None => {
631-
build_duration.observe_now(&runtime_context);
631+
build_duration.record(&runtime_context);
632632
debug!(
633633
?parent_commitment,
634634
reason = "block building failed",
@@ -638,11 +638,11 @@ where
638638
}
639639
},
640640
};
641-
build_duration.observe_now(&runtime_context);
641+
build_duration.record(&runtime_context);
642642

643643
let erasure_encode_duration = erasure_encode_duration.start(&runtime_context);
644644
let coded_block = CodedBlock::<B, C, H>::new(built_block, coding_config, &strategy);
645-
erasure_encode_duration.observe_now(&runtime_context);
645+
erasure_encode_duration.record(&runtime_context);
646646

647647
let commitment = coded_block.commitment();
648648
{

‎consensus/src/marshal/coding/shards/engine.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ where
602602
&self.strategy,
603603
)
604604
.map_err(Error::Coding)?;
605-
decode_duration.observe_now(self.context.as_ref());
605+
decode_duration.record(self.context.as_ref());
606606

607607
// Attempt to decode the block from the encoded blob
608608
let (inner, config): (B, CodingConfig) =

‎consensus/src/marshal/standard/deferred.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ where
401401
result = build_request => match result {
402402
Some(block) => block,
403403
None => {
404-
build_duration.observe_now(&runtime_context);
404+
build_duration.record(&runtime_context);
405405
debug!(
406406
?parent_digest,
407407
reason = "block building failed",
@@ -411,7 +411,7 @@ where
411411
}
412412
},
413413
};
414-
build_duration.observe_now(&runtime_context);
414+
build_duration.record(&runtime_context);
415415

416416
let digest = built_block.digest();
417417
{

‎consensus/src/marshal/standard/inline.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ where
330330
result = build_request => match result {
331331
Some(block) => block,
332332
None => {
333-
build_duration.observe_now(&runtime_context);
333+
build_duration.record(&runtime_context);
334334
debug!(
335335
?parent_digest,
336336
reason = "block building failed",
@@ -340,7 +340,7 @@ where
340340
}
341341
},
342342
};
343-
build_duration.observe_now(&runtime_context);
343+
build_duration.record(&runtime_context);
344344

345345
let digest = built_block.digest();
346346
{

‎consensus/src/ordered_broadcast/engine.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ impl<
473473
payload,
474474
result,
475475
} = verify;
476-
duration.observe_now(self.context.as_ref());
476+
duration.record(self.context.as_ref());
477477
match result {
478478
Err(err) => {
479479
warn!(?err, ?context, "verified returned error");
@@ -606,7 +606,7 @@ impl<
606606
if let Some(ref signer) = self.sequencer_signer {
607607
if chunk.sequencer == signer.public_key() {
608608
if let Some(start) = self.propose_timer.take() {
609-
start.observe_now(self.context.as_ref());
609+
start.record(self.context.as_ref());
610610
}
611611
}
612612
}

‎consensus/src/simplex/actors/batcher/actor.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -572,17 +572,17 @@ where
572572
let verified = if round.ready_notarizes() {
573573
let timer = self.verify_latency.start(self.context.as_ref());
574574
let result = round.verify_notarizes(self.context.as_mut(), &self.strategy);
575-
timer.observe_now(self.context.as_ref());
575+
timer.record(self.context.as_ref());
576576
Some(result)
577577
} else if round.ready_nullifies() {
578578
let timer = self.verify_latency.start(self.context.as_ref());
579579
let result = round.verify_nullifies(self.context.as_mut(), &self.strategy);
580-
timer.observe_now(self.context.as_ref());
580+
timer.record(self.context.as_ref());
581581
Some(result)
582582
} else if round.ready_finalizes() {
583583
let timer = self.verify_latency.start(self.context.as_ref());
584584
let result = round.verify_finalizes(self.context.as_mut(), &self.strategy);
585-
timer.observe_now(self.context.as_ref());
585+
timer.record(self.context.as_ref());
586586
Some(result)
587587
} else {
588588
None
@@ -620,30 +620,30 @@ where
620620
}
621621

622622
// Try to construct and forward certificates
623-
let notarization = self
624-
.recover_latency
625-
.time_some(self.context.as_ref(), || round.try_construct_notarization(&self.scheme, &self.strategy));
623+
let started = self.recover_latency.start(self.context.as_ref());
624+
let notarization = round.try_construct_notarization(&self.scheme, &self.strategy);
626625
if let Some(notarization) = notarization {
626+
started.record(self.context.as_ref());
627627
debug!(view = %updated_view, "constructed notarization, forwarding to voter");
628628

629629
// Forward notarization to voter
630630
voter
631631
.recovered(Certificate::Notarization(notarization))
632632
.await;
633633
}
634-
let nullification = self
635-
.recover_latency
636-
.time_some(self.context.as_ref(), || round.try_construct_nullification(&self.scheme, &self.strategy));
634+
let started = self.recover_latency.start(self.context.as_ref());
635+
let nullification = round.try_construct_nullification(&self.scheme, &self.strategy);
637636
if let Some(nullification) = nullification {
637+
started.record(self.context.as_ref());
638638
debug!(view = %updated_view, "constructed nullification, forwarding to voter");
639639
voter
640640
.recovered(Certificate::Nullification(nullification))
641641
.await;
642642
}
643-
let finalization = self
644-
.recover_latency
645-
.time_some(self.context.as_ref(), || round.try_construct_finalization(&self.scheme, &self.strategy));
643+
let started = self.recover_latency.start(self.context.as_ref());
644+
let finalization = round.try_construct_finalization(&self.scheme, &self.strategy);
646645
if let Some(finalization) = finalization {
646+
started.record(self.context.as_ref());
647647
debug!(view = %updated_view, "constructed finalization, forwarding to voter");
648648
voter
649649
.recovered(Certificate::Finalization(finalization))

‎resolver/src/p2p/engine.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ impl<
334334
self.handle_serve(&mut sender, peer, id, result, self.priority_responses)
335335
.await;
336336
if success {
337-
duration.observe_now(self.context.as_ref());
337+
duration.record(self.context.as_ref());
338338
}
339339
},
340340
// Handle network messages
@@ -429,7 +429,7 @@ impl<
429429
// Record metrics
430430
self.metrics.fetch.inc(Status::Success);
431431
let duration = self.fetch_timers.remove(&key).unwrap(); // must exist in the map
432-
duration.observe_now(self.context.as_ref());
432+
duration.record(self.context.as_ref());
433433

434434
// Clear all targets for this key
435435
self.fetcher.clear_targets(&key);

‎runtime/src/telemetry/metrics/histogram.rs‎

Lines changed: 7 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,7 @@
1717
//!
1818
//! let started = latency.start(&context);
1919
//! context.sleep(Duration::from_millis(1)).await;
20-
//! started.observe_now(&context);
21-
//!
22-
//! let value = latency.time(&context, || 7);
23-
//! assert_eq!(value, 7);
24-
//!
25-
//! let missing: Option<u8> = latency.time_some(&context, || None);
26-
//! assert!(missing.is_none());
20+
//! started.record(&context);
2721
//! });
2822
//! ```
2923
@@ -90,9 +84,9 @@ pub struct Timed {
9084

9185
/// A sampled histogram observation.
9286
///
93-
/// This token is explicit: dropping it records nothing. Call [`Started::observe_now`] or
94-
/// [`Started::observe_at`] to record a duration.
95-
#[must_use = "call observe_now/observe_at to record the timing, or drop it to skip recording"]
87+
/// This token is explicit: dropping it records nothing. Call [`Started::record`] to record
88+
/// a duration.
89+
#[must_use = "call record to record the timing, or drop it to skip recording"]
9690
pub struct Started {
9791
/// The histogram to record durations in.
9892
histogram: Histogram,
@@ -117,58 +111,16 @@ impl Timed {
117111

118112
/// Sample the current time and return an explicit observation token.
119113
pub fn start<C: Clock + ?Sized>(&self, clock: &C) -> Started {
120-
self.start_at(clock.current())
121-
}
122-
123-
/// Create an observation token from an existing start time.
124-
pub fn start_at(&self, start: SystemTime) -> Started {
125114
Started {
126115
histogram: self.histogram.clone(),
127-
start,
116+
start: clock.current(),
128117
}
129118
}
130-
131-
/// Observe the duration between two points in time directly.
132-
pub fn observe_between(&self, start: SystemTime, end: SystemTime) {
133-
self.histogram.observe_between(start, end);
134-
}
135-
136-
/// Time an operation, always recording the elapsed duration.
137-
pub fn time<C: Clock + ?Sized, T, F: FnOnce() -> T>(&self, clock: &C, f: F) -> T {
138-
let started = self.start(clock);
139-
let result = f();
140-
started.observe_now(clock);
141-
result
142-
}
143-
144-
/// Time an operation, recording only if it returns `Some`.
145-
pub fn time_some<C: Clock + ?Sized, T, F: FnOnce() -> Option<T>>(
146-
&self,
147-
clock: &C,
148-
f: F,
149-
) -> Option<T> {
150-
let started = self.start(clock);
151-
let result = f();
152-
if result.is_some() {
153-
started.observe_now(clock);
154-
}
155-
result
156-
}
157119
}
158120

159121
impl Started {
160-
/// Returns the sampled start time.
161-
pub const fn start(&self) -> SystemTime {
162-
self.start
163-
}
164-
165122
/// Record the observation against the current time of `clock`.
166-
pub fn observe_now<C: Clock + ?Sized>(self, clock: &C) {
167-
self.observe_at(clock.current());
168-
}
169-
170-
/// Record the observation against `end`.
171-
pub fn observe_at(self, end: SystemTime) {
172-
self.histogram.observe_between(self.start, end);
123+
pub fn record<C: Clock + ?Sized>(self, clock: &C) {
124+
self.histogram.observe_between(self.start, clock.current());
173125
}
174126
}

0 commit comments

Comments
 (0)