feat: move Clock::timed method to tick by schgoo · Pull Request #336 · microsoft/oxidizer

schgoo · 2026-03-24T15:33:06Z

For cachet this helper function was added to a Clock extension to time async futures and return the result and duration (primarily for telemetry). I'm proposing moving this to the tick crate.

codecov · 2026-03-24T15:37:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (a5fd76d) to head (e21eeb3).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main     #336    +/-   ##
========================================
  Coverage   100.0%   100.0%            
========================================
  Files         187      210    +23     
  Lines       14618    15514   +896     
========================================
+ Hits        14618    15514   +896

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Moves an async timing helper from cachet’s telemetry extension into the shared tick crate, making it available as an inherent Clock::timed API for measuring future execution duration (useful for telemetry and tests with controlled time).

Changes:

Added tick::Timed / tick::TimedResult and implemented Clock::timed.
Added tests and an example demonstrating the timing API.
Updated cachet to use Clock::timed and removed the old telemetry extension module.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/tick/src/timed.rs	New `Timed` future + `TimedResult` output type for measuring elapsed time.
crates/tick/src/lib.rs	Wires the new module and re-exports `Timed` / `TimedResult`.
crates/tick/src/clock.rs	Adds `Clock::timed` and associated tests.
crates/tick/examples/timed_result.rs	Adds a runnable example using `Clock::timed`.
crates/tick/Cargo.toml	Registers the new example (Tokio-gated).
crates/cachet/src/wrapper.rs	Switches from `timed_async` extension to `Clock::timed`.
crates/cachet/src/telemetry/mod.rs	Removes telemetry `ext` module from the module tree.
crates/cachet/src/telemetry/ext.rs	Deletes the old `ClockExt`/`Timed` implementation previously local to `cachet`.
crates/cachet/src/refresh.rs	Switches to `Clock::timed`.
crates/cachet/src/fallback.rs	Switches to `Clock::timed` (including promotion insert timing).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T15:38:46Z

+    pub fn timed<F>(&self, f: F) -> Timed<F>
+    where
+        F: Future,
+    {


Clock::timed uses F: Future, but Future isn’t in scope in this module (no use std::future::Future; and it’s not in the prelude). This will not compile as-is; import std::future::Future or fully qualify the bound.

Copilot · 2026-03-24T15:38:47Z

+/// use std::time::Duration;
+///


The doctest imports std::time::Duration but doesn’t use it. This produces an unused_imports warning when running doctests; consider removing the import to keep the example warning-free.

Suggested change

/// use std::time::Duration;

///

Copilot · 2026-03-24T15:38:47Z

+    /// use std::time::Duration;
+    ///


This doc example imports std::time::Duration but doesn’t use it, which causes an unused_imports warning in doctest compilation. Consider removing the unused import so the example stays warning-free.

Suggested change

/// use std::time::Duration;

///

Copilot · 2026-03-24T15:38:48Z

+impl<F: Future> Future for Timed<F> {
+    type Output = TimedResult<F::Output>;


Timed's Future implementation uses the Future trait (impl<F: Future> Future for Timed<F>) but Future isn’t imported or fully qualified, which will fail to compile. Add use std::future::Future; (or fully qualify std::future::Future in both places).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T10:40:44Z

+    #[tokio::test]
+    async fn timed_handles_pending() {
+        use std::pin::Pin;
+        use std::sync::Arc;


This new #[tokio::test] is missing #[cfg_attr(miri, ignore)]. CI runs Miri for the workspace, and Tokio-based async tests are typically ignored under Miri elsewhere in this file.

Copilot · 2026-03-25T10:40:45Z

-                .clock
-                .timed_async(self.inner.primary.insert(key.clone(), v.clone()))
-                .await;
+            let timed_insert = self.inner.clock.timed(self.inner.primary.insert(key.clone(), v.clone())).await;


This statement is formatted as a single very long line. Please run rustfmt (or break the call across lines) to match the project's formatting expectations and avoid rustfmt CI failures.

Suggested change

let timed_insert = self.inner.clock.timed(self.inner.primary.insert(key.clone(), v.clone())).await;

let timed_insert = self

.inner

.clock

.timed(self.inner.primary.insert(key.clone(), v.clone()))

.await;

Copilot · 2026-03-25T10:40:45Z

+    ///
+    /// # async fn timed_example(clock: &Clock) {
+    /// let TimedResult { result, duration } = clock.timed(async { 42 }).await;
+    /// assert_eq!(result, 42);


The doctest example introduces unused bindings/imports (Duration import and the duration binding from TimedResult). This can produce warnings in doctest builds; consider removing the unused import and/or using _duration (or asserting on duration) to keep the example warning-free.

Suggested change

/// assert_eq!(result, 42);

/// assert_eq!(result, 42);

/// assert!(duration >= Duration::from_millis(0));

Copilot · 2026-03-25T10:40:45Z

+/// use std::time::Duration;
+///
+/// use tick::{Clock, TimedResult};
+///
+/// # async fn example(clock: &Clock) {
+/// let TimedResult { result, duration } = clock.timed(async { 42 }).await;


This doctest example has unused items (the Duration import and the duration binding). Consider removing the unused import and/or renaming duration to _duration (or asserting on it) so the example stays warning-free when compiled as a doctest.

Suggested change

/// use std::time::Duration;

///

/// use tick::{Clock, TimedResult};

///

/// # async fn example(clock: &Clock) {

/// let TimedResult { result, duration } = clock.timed(async { 42 }).await;

/// use tick::{Clock, TimedResult};

///

/// # async fn example(clock: &Clock) {

/// let TimedResult { result, duration: _duration } = clock.timed(async { 42 }).await;

Copilot · 2026-03-25T10:40:45Z

+    #[tokio::test]
+    async fn timed_measures_duration() {
+        let control = ClockControl::new();
+        let clock = control.to_clock();


This new #[tokio::test] is not annotated with #[cfg_attr(miri, ignore)], unlike other Tokio-based tests in this module. Since CI runs cargo miri test --all-features, this test should be ignored under Miri (or rewritten to avoid Tokio).

psandana · 2026-03-25T15:22:23Z

 ///
 /// # async fn example(clock: &Clock) {
 /// let TimedResult { result, duration } = clock.timed(async { 42 }).await;
+/// println!("Result: {}, Duration: {:?}", result, duration);


Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T21:42:08Z

+    /// Created via [`Clock::timed`][crate::Clock::timed].
+    #[must_use = "futures do nothing unless polled"]
+    pub struct Timed<F> {
+        #[pin]
+        pub(crate) inner: F,
+        pub(crate) watch: Stopwatch,
+    }


Timed is a public type but doesn’t implement Debug. With the workspace missing_debug_implementations = "warn" lint (and CI running with -D warnings), this will fail the build. Derive/implement Debug for Timed (matching other public futures like Timeout).

Copilot · 2026-03-27T21:42:09Z

-pub(crate) mod ext;
 #[cfg(any(feature = "metrics", test))]
 pub(crate) mod metrics;



With telemetry::ext removed, cachet no longer appears to use pin-project-lite anywhere. Because CI runs cargo clippy --all-targets --all-features -- -D warnings and the workspace enables Clippy’s cargo lint group, this is likely to trigger clippy::unused_crate_dependencies. Remove pin-project-lite from crates/cachet/Cargo.toml (or add an explicit allow if it’s intentionally kept).

Suggested change

// Ensure `pin-project-lite` is considered used so that

// `clippy::unused_crate_dependencies` does not trigger for this crate.

use pin_project_lite as _;

martintmk · 2026-03-30T14:29:57Z

Personally, I don't see much value here over just using Stopwatch + regular async execution, but I won't block merging this if you feel this is valuable.

sandersaares · 2026-03-31T05:12:36Z

+/// # }
+/// ```
+#[derive(Debug, Clone, Copy)]
+pub struct TimedResult<R> {


A type named XyzResult that is not a type alias for std::result::Result is confusing - "results" in Rust tend to be fairly specifically type alises of std::result::Result with scenario-specific type parameters. We should avoid the word "result" when we are simply referring to outputs or return values.

Suggested name TimedOutput to align with Future<Output = ...> naming.

sandersaares · 2026-03-31T05:16:18Z

+
+    // Start some background work that returns a result after a delay.
+    let background_job = async {
+        clock.delay(std::time::Duration::from_millis(10)).await;


This is an oddly verbose type path. Do we really need std::time::Duration spelled out? Would be more readable if we used some more use statements.

sandersaares · 2026-03-31T05:23:10Z

Personally, I don't see much value here over just using Stopwatch + regular async execution, but I won't block merging this if you feel this is valuable.

I share the feeling of this being low-impact code but at the same time, it seems to be marginally useful where it is used here. With Rust we also have the fact that the async fn generator is somewhat limited and we have to hand-roll futures sometimes, at which point having a pre-made building block like this can be valuable. Even a three-liner becomes a headache if you have to hand-roll futures! Not the case here but I can imagine it being the case somewhere if I stretch my imagination.

If we do think of this as "maybe not worth spending the compile time on", would a tick_extensions crate make sense for this and similar opt-in features? I am unsure how useful that is as an approach - reminds me of futures where futures-util is actually the most useful crate these days, and hyper-util where a lot of the important Hyper types live. If everyone will use it anyway in their dependency tree, no point splitting off? [Well maybe parallelism still helps compile times?]

schgoo added 2 commits March 24, 2026 11:15

Move timed_async to tick

6eb159e

Add documentation

2ff96e0

Copilot AI review requested due to automatic review settings March 24, 2026 15:33

Copilot started reviewing on behalf of schgoo March 24, 2026 15:34 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

psandana approved these changes Mar 24, 2026

View reviewed changes

Update crates/tick/src/timed.rs

a315a97

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 25, 2026 10:35

Copilot started reviewing on behalf of schgoo March 25, 2026 10:35 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Fix doctests, resolve clippy and comments

05a53ab

psandana reviewed Mar 25, 2026

View reviewed changes

psandana approved these changes Mar 25, 2026

View reviewed changes

Fix comments/clippy

e21eeb3

Copilot AI review requested due to automatic review settings March 27, 2026 21:31

Copilot started reviewing on behalf of schgoo March 27, 2026 21:35 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

sandersaares requested changes Mar 31, 2026

View reviewed changes

		impl<F: Future> Future for Timed<F> {
		type Output = TimedResult<F::Output>;

	/// assert_eq!(result, 42);
	/// assert_eq!(result, 42);
	/// assert!(duration >= Duration::from_millis(0));

+// Ensure `pin-project-lite` is considered used so that
+// `clippy::unused_crate_dependencies` does not trigger for this crate.
+use pin_project_lite as _;

Conversation

schgoo commented Mar 24, 2026

Uh oh!

codecov Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

psandana Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

martintmk commented Mar 30, 2026

Uh oh!

sandersaares Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

sandersaares Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

sandersaares commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Mar 24, 2026 •

edited

Loading

sandersaares commented Mar 31, 2026 •

edited

Loading