From 46c179bbbec79f480ecf2851bbea54cb73388248 Mon Sep 17 00:00:00 2001 From: Patrice Billaut <57354406+pbillaut@users.noreply.github.com> Date: Thu, 19 Sep 2024 23:17:01 +0200 Subject: [PATCH] perf(account): reduce memory footprint (#17) --- .gitignore | 4 ++++ Cargo.toml | 5 +++++ Makefile.toml | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/account.rs | 49 +++++++++++++++++++++++-------------------------- 4 files changed, 77 insertions(+), 26 deletions(-) create mode 100644 Makefile.toml diff --git a/.gitignore b/.gitignore index 4768aef..dabe4cd 100644 --- a/.gitignore +++ b/.gitignore @@ -2,5 +2,9 @@ .idea/ +# Profiling Tools +heaptrack*.zst +!docs/**/heaptrack*.zst + flamegraph.svg profile.json diff --git a/Cargo.toml b/Cargo.toml index da3b054..4e00c71 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,11 @@ tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } criterion = { version = "0.5.1", features = ["html_reports"] } test-log = { version = "0.2.16", features = ["trace"] } +[profile.perf] +# Profile for memory profiling +inherits = "release" +debug = true + [[bench]] name = "bench_main" harness = false \ No newline at end of file diff --git a/Makefile.toml b/Makefile.toml new file mode 100644 index 0000000..25561dd --- /dev/null +++ b/Makefile.toml @@ -0,0 +1,45 @@ +[env] +BENCHES_DATA_DIR = "${CARGO_MAKE_WORKING_DIRECTORY}/benches/data" +BUILD_PROFILE_PERF = "perf" + +# ------------------------------------------------------ +# Group: Memory Profiling +# ------------------------------------------------------ +[tasks.build-perf] +description = "Build the perf release to profile against" +command = "cargo" +args = ["build", "--profile", "${BUILD_PROFILE_PERF}"] + +[tasks.heaptrack] +description = "Run heaptrack" +dependencies = ["build-perf"] +script = ''' +SIZE="${1:-10K}" +TAG="${2:-$(date +"%Y-%m-%d_%H-%M-%S")}" + +if [ -z "${SIZE}" ] || [ -z "${TAG}" ]; then + echo "ERROR: Invalid or missing argument" + echo + echo "Usage: cargo make mem-perf " + echo "Example: cargo make mem-perf 1K optimized" + echo + exit 1 +fi + +SAMPLE_FILE="${BENCHES_DATA_DIR}/activities_${SIZE}.csv" +if [ ! -f "$SAMPLE_FILE" ]; then + echo "ERROR: File '${SAMPLE_FILE}' does not exist." + echo + echo "Available sample files in '${DATA_DIR}':" + for file in ${BENCHES_DATA_DIR}/activities_*.csv; do + echo "• $(basename "$file")" + done + echo + exit 1 +fi + +OUTPUT_FILE="${CARGO_MAKE_WORKING_DIRECTORY}/heaptrack.${CARGO_MAKE_CRATE_NAME}.${SIZE}.${TAG}" +BINARY="${CARGO_MAKE_CRATE_TARGET_DIRECTORY}/${BUILD_PROFILE_PERF}/${CARGO_MAKE_CRATE_NAME}" + +heaptrack --output "${OUTPUT_FILE}" "${BINARY}" "${SAMPLE_FILE}" +''' diff --git a/src/account.rs b/src/account.rs index 5910d0f..739f237 100644 --- a/src/account.rs +++ b/src/account.rs @@ -6,7 +6,7 @@ use crate::ClientID; use rust_decimal::Decimal; use rust_decimal_macros::dec; use std::collections::hash_map::Entry; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; /// An abstraction over the balances of a client. /// @@ -56,10 +56,10 @@ pub struct Account { locked: bool, #[serde(skip)] - dispute_cases: HashMap, + dispute_cases: HashSet, #[serde(skip)] - transaction_record: HashMap, + transaction_record: HashMap, } impl Account { @@ -70,7 +70,7 @@ impl Account { total: dec!(0.0), available: dec!(0.0), locked: false, - dispute_cases: HashMap::new(), + dispute_cases: HashSet::new(), transaction_record: HashMap::new(), } } @@ -152,43 +152,40 @@ impl Account { } fn initiate_dispute(&mut self, transaction_id: TransactionID) -> AccountActivityResult<()> { - match self.dispute_cases.entry(transaction_id) { - Entry::Occupied(_) => Err(FailedDisputeCase("transaction already disputed".into())), - Entry::Vacant(entry) => { - match self.transaction_record.get(&transaction_id) { - None => Ok(()), - Some(transaction) => { - entry.insert(transaction.amount()); - self.hold(transaction.amount()) - } + match self.dispute_cases.contains(&transaction_id) { + true => Err(FailedDisputeCase("transaction already disputed".into())), + false => match self.transaction_record.get(&transaction_id) { + None => Ok(()), + Some(&amount) => { + self.dispute_cases.insert(transaction_id); + self.hold(amount) } } } } fn resolve_dispute(&mut self, transaction_id: &TransactionID) -> AccountActivityResult<()> { - match self.dispute_cases.remove(transaction_id) { - None => Ok(()), - Some(amount) => self.release(amount), + if let Some(&amount) = self.transaction_record.get(transaction_id) { + self.release(amount)?; + self.dispute_cases.remove(transaction_id); } + Ok(()) } fn issue_chargeback(&mut self, transaction_id: &TransactionID) -> AccountActivityResult<()> { - match self.dispute_cases.remove(transaction_id) { - None => Ok(()), - Some(amount) => { - self.charge(amount)?; - self.lock(); - Ok(()) - } + if let Some(&amount) = self.transaction_record.get(transaction_id) { + self.charge(amount)?; + self.dispute_cases.remove(transaction_id); + self.lock(); } + Ok(()) } fn record_transaction(&mut self, transaction: Transaction) -> AccountActivityResult<()> { match self.transaction_record.entry(transaction.id()) { Entry::Occupied(_) => Err(FailedTransaction("transaction already recorded".into())), Entry::Vacant(entry) => { - entry.insert(transaction); + entry.insert(transaction.amount()); Ok(()) } } @@ -227,7 +224,7 @@ pub mod test_utils { use crate::ClientID; use rust_decimal::Decimal; use rust_decimal_macros::dec; - use std::collections::HashMap; + use std::collections::{HashMap, HashSet}; pub enum LockStatus { Locked, @@ -251,7 +248,7 @@ pub mod test_utils { LockStatus::Locked => true, LockStatus::Unlocked => false, }, - dispute_cases: HashMap::new(), + dispute_cases: HashSet::new(), transaction_record: HashMap::new(), } }