From 9a7b681f0c2f1df33840ae8c50cb0d5146e3b835 Mon Sep 17 00:00:00 2001
From: steviez <steven@solana.com>
Date: Sun, 19 Nov 2023 23:05:32 -0600
Subject: [PATCH] Remove key_size() method from Column trait (#34021)

This helper simply called std::mem::size_of<Self::Index>(). However, all
of the underlying functions that create keys manually copy fields into a
byte array. The fields are copied in end-to-end whereas size_of() might
include alignment bytes.

For example, a (u64, u32) only has 12 bytes of "data", but it would
have size 16 due to the 4 alignment padding bytes that would be
added to get the u32 (size 4) aligned with the u64 (size 8).
---
 ledger-tool/src/main.rs     | 24 +++++++++++++++---------
 ledger/src/blockstore_db.rs |  4 ----
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs
index 98f8fc797253fa..a5b979c2ff35da 100644
--- a/ledger-tool/src/main.rs
+++ b/ledger-tool/src/main.rs
@@ -743,17 +743,23 @@ fn analyze_column<
     db: &Database,
     name: &str,
 ) {
+    let mut key_len: u64 = 0;
     let mut key_tot: u64 = 0;
     let mut val_hist = histogram::Histogram::new();
     let mut val_tot: u64 = 0;
     let mut row_hist = histogram::Histogram::new();
-    let a = C::key_size() as u64;
-    for (_x, y) in db.iter::<C>(blockstore_db::IteratorMode::Start).unwrap() {
-        let b = y.len() as u64;
-        key_tot += a;
-        val_hist.increment(b).unwrap();
-        val_tot += b;
-        row_hist.increment(a + b).unwrap();
+    for (key, val) in db.iter::<C>(blockstore_db::IteratorMode::Start).unwrap() {
+        // Key length is fixed, only need to calculate it once
+        if key_len == 0 {
+            key_len = C::key(key).len() as u64;
+        }
+        let val_len = val.len() as u64;
+
+        key_tot += key_len;
+        val_hist.increment(val_len).unwrap();
+        val_tot += val_len;
+
+        row_hist.increment(key_len + val_len).unwrap();
     }
 
     let json_result = if val_hist.entries() > 0 {
@@ -761,7 +767,7 @@ fn analyze_column<
             "column":name,
             "entries":val_hist.entries(),
             "key_stats":{
-                "max":a,
+                "max":key_len,
                 "total_bytes":key_tot,
             },
             "val_stats":{
@@ -790,7 +796,7 @@ fn analyze_column<
         "column":name,
         "entries":val_hist.entries(),
         "key_stats":{
-            "max":a,
+            "max":key_len,
             "total_bytes":0,
         },
         "val_stats":{
diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs
index 3e1d8812f61ea2..87b26ce4d5b1a9 100644
--- a/ledger/src/blockstore_db.rs
+++ b/ledger/src/blockstore_db.rs
@@ -712,10 +712,6 @@ impl Rocks {
 pub trait Column {
     type Index;
 
-    fn key_size() -> usize {
-        std::mem::size_of::<Self::Index>()
-    }
-
     fn key(index: Self::Index) -> Vec<u8>;
     fn index(key: &[u8]) -> Self::Index;
     // This trait method is primarily used by `Database::delete_range_cf()`, and is therefore only