From 932a96051e92a5111d93a840ea8ccae2d05b8891 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Wed, 27 Sep 2023 14:38:24 +0000
Subject: [PATCH] feat: make it possible to construct a GasDuration (#1898)

This lets us construct "gas charge" objects with constant times (for
tracing).

- Use an explicit enum with 3 options to avoid allocating/using an
  Arc+OnceCell when we don't need it.
- Implement Debug manually to hide implementation details.
- Change the assertion inside `GasTimer::new` to "debug only". If
  someone passes a "filled" duration, treat the gas timer as a no-op.
---
 fvm/src/gas/timer.rs | 46 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/fvm/src/gas/timer.rs b/fvm/src/gas/timer.rs
index 3684cd0a0..e3e097caf 100644
--- a/fvm/src/gas/timer.rs
+++ b/fvm/src/gas/timer.rs
@@ -1,3 +1,4 @@
+use std::fmt;
 // Copyright 2021-2023 Protocol Labs
 // SPDX-License-Identifier: Apache-2.0, MIT
 use std::sync::Arc;
@@ -13,12 +14,38 @@ type DurationCell = Arc<OnceCell<Duration>>;
 ///
 /// This is normally created with an empty inner, because at the point of creation
 /// we don't know if tracing is on or not. It will be filled in later by `GasTimer`.
-#[derive(Default, Debug, Clone)]
-pub struct GasDuration(Option<DurationCell>);
+#[derive(Default, Clone)]
+pub struct GasDuration(GasDurationInner);
+
+#[derive(Default, Clone)]
+pub enum GasDurationInner {
+    #[default]
+    None,
+    Atomic(DurationCell),
+    Constant(Duration),
+}
 
 impl GasDuration {
     pub fn get(&self) -> Option<&Duration> {
-        self.0.as_ref().and_then(|d| d.get())
+        match &self.0 {
+            GasDurationInner::None => None,
+            GasDurationInner::Atomic(d) => d.get(),
+            GasDurationInner::Constant(d) => Some(d),
+        }
+    }
+}
+
+impl From<Duration> for GasDuration {
+    fn from(d: Duration) -> Self {
+        GasDuration(GasDurationInner::Constant(d))
+    }
+}
+
+impl fmt::Debug for GasDuration {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_tuple("GasDuration")
+            .field(&self.get() as &dyn fmt::Debug)
+            .finish()
     }
 }
 
@@ -53,16 +80,19 @@ impl GasTimer {
     /// Create a new timer that will update the elapsed time of a charge when it's finished.
     ///
     /// As a side effect it will establish the cell in the `GasDuration`, if it has been empty so far.
+    ///
+    /// When compiled in debug mode, passing a "filled" duration will panic.
     pub fn new(duration: &mut GasDuration) -> Self {
-        assert!(duration.get().is_none(), "GasCharge::elapsed already set!");
-
+        debug_assert!(duration.get().is_none(), "GasCharge::elapsed already set!");
         let cell = match &duration.0 {
-            Some(cell) => cell.clone(),
-            None => {
+            GasDurationInner::None => {
                 let cell = DurationCell::default();
-                duration.0 = Some(cell.clone());
+                duration.0 = GasDurationInner::Atomic(cell.clone());
                 cell
             }
+            GasDurationInner::Atomic(cell) if cell.get().is_none() => cell.clone(),
+            // If the duration has already been set, the timer is a no-op.
+            _ => return Self(None),
         };
 
         Self(Some(GasTimerInner {