[stress tests] simplify Payload trait to make it easier to add new st…

…ress-test workloads - Remove `get_object_id` function + `new_object` and `new_gas` arguments to `make_new_paylod` - Derive `Debug` instead of hand-rolling
Myrilandel · Mar 1, 2023 · 7a47320 · 7a47320
1 parent 7b60006
commit 7a47320
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 87 deletions.
diff --git a/crates/sui-benchmark/src/drivers/bench_driver.rs b/crates/sui-benchmark/src/drivers/bench_driver.rs
@@ -356,9 +356,6 @@ impl Driver<(BenchmarkStats, StressStats)> for BenchDriver {
                                     .then(|res| async move  {
                                         match res {
                                             Ok(effects) => {
-                                                let new_version = effects.mutated().iter().find(|(object_ref, _)| {
-                                                    object_ref.0 == b.1.get_object_id()
-                                                }).map(|x| x.0).unwrap();
                                                 let latency = start.elapsed();
                                                 metrics_cloned.latency_s.with_label_values(&[&b.1.get_workload_type().to_string()]).observe(latency.as_secs_f64());
                                                 metrics_cloned.num_success.with_label_values(&[&b.1.get_workload_type().to_string()]).inc();
@@ -370,7 +367,7 @@ impl Driver<(BenchmarkStats, StressStats)> for BenchDriver {
                                                 }
                                                 NextOp::Response(Some((
                                                     latency,
-                                                    b.1.make_new_payload(new_version, effects.gas_object().0, &effects),
+                                                    b.1.make_new_payload(&effects),
                                                 ),
                                                 ))
                                             }
@@ -404,9 +401,6 @@ impl Driver<(BenchmarkStats, StressStats)> for BenchDriver {
                                 .then(|res| async move {
                                     match res {
                                         Ok(effects) => {
-                                            let new_version = effects.mutated().iter().find(|(object_ref, _)| {
-                                                object_ref.0 == payload.get_object_id()
-                                            }).map(|x| x.0).unwrap();
                                             let latency = start.elapsed();
                                             metrics_cloned.latency_s.with_label_values(&[&payload.get_workload_type().to_string()]).observe(latency.as_secs_f64());
                                             metrics_cloned.num_success.with_label_values(&[&payload.get_workload_type().to_string()]).inc();
@@ -416,7 +410,7 @@ impl Driver<(BenchmarkStats, StressStats)> for BenchDriver {
                                             if let Some(sig_info) = effects.quorum_sig() { sig_info.authorities(&committee_cloned).for_each(|name| metrics_cloned.validators_in_effects_cert.with_label_values(&[&name.unwrap().to_string()]).inc()) }
                                             NextOp::Response(Some((
                                                 latency,
-                                                payload.make_new_payload(new_version, effects.gas_object().0, &effects),
+                                                payload.make_new_payload(&effects),
                                             )))
                                         }
                                         Err(err) => {

diff --git a/crates/sui-benchmark/src/workloads/delegation.rs b/crates/sui-benchmark/src/workloads/delegation.rs
@@ -10,7 +10,7 @@ use async_trait::async_trait;
 use rand::seq::IteratorRandom;
 use std::sync::Arc;
 use sui_core::test_utils::make_transfer_sui_transaction;
-use sui_types::base_types::{ObjectID, ObjectRef, SuiAddress};
+use sui_types::base_types::{ObjectRef, SuiAddress};
 use sui_types::crypto::{get_key_pair, AccountKeyPair};
 use sui_types::messages::VerifiedTransaction;
 use test_utils::messages::make_delegation_transaction;
@@ -50,37 +50,24 @@ impl Payload for DelegationTestPayload {
         }
     }
 
-    fn make_new_payload(
-        self: Box<Self>,
-        _: ObjectRef,
-        new_gas: ObjectRef,
-        effects: &ExecutionEffects,
-    ) -> Box<dyn Payload> {
+    fn make_new_payload(self: Box<Self>, effects: &ExecutionEffects) -> Box<dyn Payload> {
         let coin = match self.coin {
             None => Some(effects.created().get(0).unwrap().0),
             Some(_) => None,
         };
         Box::new(DelegationTestPayload {
             coin,
-            gas: new_gas,
+            gas: effects.gas_object().0,
             validator: self.validator,
             sender: self.sender,
             keypair: self.keypair,
             system_state_observer: self.system_state_observer,
         })
     }
 
-    fn get_object_id(&self) -> ObjectID {
-        self.gas.0
-    }
-
     fn get_workload_type(&self) -> WorkloadType {
         WorkloadType::Delegation
     }
-
-    fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:?}", self as &DelegationTestPayload)
-    }
 }
 
 pub struct DelegationWorkload;

diff --git a/crates/sui-benchmark/src/workloads/payload.rs b/crates/sui-benchmark/src/workloads/payload.rs
@@ -1,8 +1,6 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use sui_types::base_types::{ObjectID, ObjectRef};
-
 use sui_types::messages::VerifiedTransaction;
 
 use crate::ExecutionEffects;
@@ -11,24 +9,10 @@ use rand_distr::WeightedAliasIndex;
 
 use crate::workloads::workload::WorkloadType;
 
-pub trait Payload: Send + Sync {
-    fn make_new_payload(
-        self: Box<Self>,
-        new_object: ObjectRef,
-        new_gas: ObjectRef,
-        effects: &ExecutionEffects,
-    ) -> Box<dyn Payload>;
+pub trait Payload: Send + Sync + std::fmt::Debug {
+    fn make_new_payload(self: Box<Self>, effects: &ExecutionEffects) -> Box<dyn Payload>;
     fn make_transaction(&self) -> VerifiedTransaction;
-    fn get_object_id(&self) -> ObjectID;
     fn get_workload_type(&self) -> WorkloadType;
-
-    fn debug(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result;
-}
-
-impl std::fmt::Debug for dyn Payload {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        self.debug(f)
-    }
 }
 
 #[derive(Debug)]
@@ -40,16 +24,11 @@ pub struct CombinationPayload {
 }
 
 impl Payload for CombinationPayload {
-    fn make_new_payload(
-        self: Box<Self>,
-        new_object: ObjectRef,
-        new_gas: ObjectRef,
-        effects: &ExecutionEffects,
-    ) -> Box<dyn Payload> {
+    fn make_new_payload(self: Box<Self>, effects: &ExecutionEffects) -> Box<dyn Payload> {
         let mut new_payloads = vec![];
         for (pos, e) in self.payloads.into_iter().enumerate() {
             if pos == self.curr_index {
-                let updated = e.make_new_payload(new_object, new_gas, effects);
+                let updated = e.make_new_payload(effects);
                 new_payloads.push(updated);
             } else {
                 new_payloads.push(e);
@@ -68,18 +47,10 @@ impl Payload for CombinationPayload {
         let curr = self.payloads.get(self.curr_index).unwrap();
         curr.make_transaction()
     }
-    fn get_object_id(&self) -> ObjectID {
-        let curr = self.payloads.get(self.curr_index).unwrap();
-        curr.get_object_id()
-    }
     fn get_workload_type(&self) -> WorkloadType {
         self.payloads
             .get(self.curr_index)
             .unwrap()
             .get_workload_type()
     }
-
-    fn debug(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(f, "{:?}", self as &CombinationPayload)
-    }
 }
diff --git a/crates/sui-benchmark/src/workloads/shared_counter.rs b/crates/sui-benchmark/src/workloads/shared_counter.rs
@@ -35,17 +35,12 @@ pub struct SharedCounterTestPayload {
 }
 
 impl Payload for SharedCounterTestPayload {
-    fn make_new_payload(
-        self: Box<Self>,
-        _: ObjectRef,
-        new_gas: ObjectRef,
-        _: &ExecutionEffects,
-    ) -> Box<dyn Payload> {
+    fn make_new_payload(self: Box<Self>, effects: &ExecutionEffects) -> Box<dyn Payload> {
         Box::new(SharedCounterTestPayload {
             package_id: self.package_id,
             counter_id: self.counter_id,
             counter_initial_shared_version: self.counter_initial_shared_version,
-            gas: (new_gas, self.gas.1, self.gas.2),
+            gas: (effects.gas_object().0, self.gas.1, self.gas.2),
             system_state_observer: self.system_state_observer,
         })
     }
@@ -63,16 +58,10 @@ impl Payload for SharedCounterTestPayload {
             Some(*self.system_state_observer.reference_gas_price.borrow()),
         )
     }
-    fn get_object_id(&self) -> ObjectID {
-        self.counter_id
-    }
+
     fn get_workload_type(&self) -> WorkloadType {
         WorkloadType::SharedCounter
     }
-
-    fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:?}", self as &SharedCounterTestPayload)
-    }
 }
 
 #[derive(Debug)]

diff --git a/crates/sui-benchmark/src/workloads/transfer_object.rs b/crates/sui-benchmark/src/workloads/transfer_object.rs
@@ -8,7 +8,7 @@ use std::collections::HashMap;
 use std::sync::Arc;
 
 use sui_types::{
-    base_types::{ObjectID, ObjectRef, SuiAddress},
+    base_types::{ObjectRef, SuiAddress},
     crypto::{get_key_pair, AccountKeyPair},
     messages::VerifiedTransaction,
     object::Owner,
@@ -32,12 +32,7 @@ pub struct TransferObjectTestPayload {
 }
 
 impl Payload for TransferObjectTestPayload {
-    fn make_new_payload(
-        self: Box<Self>,
-        new_object: ObjectRef,
-        new_gas: ObjectRef,
-        _: &ExecutionEffects,
-    ) -> Box<dyn Payload> {
+    fn make_new_payload(self: Box<Self>, effects: &ExecutionEffects) -> Box<dyn Payload> {
         let recipient = self
             .gas
             .iter()
@@ -49,14 +44,23 @@ impl Payload for TransferObjectTestPayload {
             .into_iter()
             .map(|x| {
                 if x.1.get_owner_address().unwrap() == self.transfer_from {
-                    (new_gas, Owner::AddressOwner(self.transfer_from), x.2)
+                    (
+                        effects.gas_object().0,
+                        Owner::AddressOwner(self.transfer_from),
+                        x.2,
+                    )
                 } else {
                     x
                 }
             })
             .collect();
         Box::new(TransferObjectTestPayload {
-            transfer_object: new_object,
+            transfer_object: effects
+                .mutated()
+                .iter()
+                .find(|(object_ref, _)| object_ref.0 == self.transfer_object.0)
+                .map(|x| x.0)
+                .unwrap(),
             transfer_from: self.transfer_to,
             transfer_to: recipient.get_owner_address().unwrap(),
             gas: updated_gas,
@@ -78,16 +82,9 @@ impl Payload for TransferObjectTestPayload {
             Some(*self.system_state_observer.reference_gas_price.borrow()),
         )
     }
-    fn get_object_id(&self) -> ObjectID {
-        self.transfer_object.0
-    }
     fn get_workload_type(&self) -> WorkloadType {
         WorkloadType::TransferObject
     }
-
-    fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:?}", self as &TransferObjectTestPayload)
-    }
 }
 
 #[derive(Debug)]