From 5e50b386f83b86359b5ecdea01b4cdef26f94afc Mon Sep 17 00:00:00 2001
From: HaoranYi <haoran.yi@solana.com>
Date: Fri, 14 Jun 2024 16:08:45 +0000
Subject: [PATCH] no sort for pack

---
 accounts-db/src/accounts_db.rs         | 22 ++++++++++++++--------
 accounts-db/src/ancient_append_vecs.rs | 26 +++++++++++---------------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs
index ba0b519e705b92..dbea882b704866 100644
--- a/accounts-db/src/accounts_db.rs
+++ b/accounts-db/src/accounts_db.rs
@@ -4396,11 +4396,9 @@ impl AccountsDb {
 
     /// return all slots that are more than one epoch old and thus could already be an ancient append vec
     /// or which could need to be combined into a new or existing ancient append vec
-    /// offset is used to combine newer slots than we normally would. This is designed to be used for testing.
-    fn get_sorted_potential_ancient_slots(&self, oldest_non_ancient_slot: Slot) -> Vec<Slot> {
-        let mut ancient_slots = self.get_roots_less_than(oldest_non_ancient_slot);
-        ancient_slots.sort_unstable();
-        ancient_slots
+    /// offset is used to combine newer slots than we normally would.
+    fn get_potential_ancient_slots(&self, oldest_non_ancient_slot: Slot) -> Vec<Slot> {
+        self.get_roots_less_than(oldest_non_ancient_slot)
     }
 
     /// get a sorted list of slots older than an epoch
@@ -4412,11 +4410,13 @@ impl AccountsDb {
 
         let oldest_non_ancient_slot = self.get_oldest_non_ancient_slot(epoch_schedule);
         let can_randomly_shrink = true;
-        let sorted_slots = self.get_sorted_potential_ancient_slots(oldest_non_ancient_slot);
+        let mut ancient_slots = self.get_potential_ancient_slots(oldest_non_ancient_slot);
         if self.create_ancient_storage == CreateAncientStorage::Append {
-            self.combine_ancient_slots(sorted_slots, can_randomly_shrink);
+            // 'Append' require that 'ancient_slots' to be sorted.
+            ancient_slots.sort_unstable();
+            self.combine_ancient_slots(ancient_slots, can_randomly_shrink);
         } else {
-            self.combine_ancient_slots_packed(sorted_slots, can_randomly_shrink);
+            self.combine_ancient_slots_packed(ancient_slots, can_randomly_shrink);
         }
     }
 
@@ -9774,6 +9774,12 @@ pub mod tests {
         fn get_storage_for_slot(&self, slot: Slot) -> Option<Arc<AccountStorageEntry>> {
             self.storage.get_slot_storage_entry(slot)
         }
+
+        fn get_sorted_potential_ancient_slots(&self, oldest_non_ancient_slot: Slot) -> Vec<Slot> {
+            let mut ancient_slots = self.get_potential_ancient_slots(oldest_non_ancient_slot);
+            ancient_slots.sort_unstable();
+            ancient_slots
+        }
     }
 
     /// this tuple contains slot info PER account
diff --git a/accounts-db/src/ancient_append_vecs.rs b/accounts-db/src/ancient_append_vecs.rs
index acf5f7847500e1..a7c4b51389d4ab 100644
--- a/accounts-db/src/ancient_append_vecs.rs
+++ b/accounts-db/src/ancient_append_vecs.rs
@@ -138,14 +138,17 @@ impl AncientSlotInfos {
         self.filter_by_smallest_capacity(tuning);
     }
 
-    // sort 'shrink_indexes' by most bytes saved, highest to lowest
+    // sort 'shrink_indexes' by most bytes saved descending (i.e. highest to lowest)
+    // then sort by slots ascending.
     fn sort_shrink_indexes_by_bytes_saved(&mut self) {
         self.shrink_indexes.sort_unstable_by(|l, r| {
             let amount_shrunk = |index: &usize| {
                 let item = &self.all_infos[*index];
                 item.capacity - item.alive_bytes
             };
-            amount_shrunk(r).cmp(&amount_shrunk(l))
+            amount_shrunk(r)
+                .cmp(&amount_shrunk(l))
+                .then(self.all_infos[*l].slot.cmp(&self.all_infos[*r].slot))
         });
     }
 
@@ -267,11 +270,7 @@ impl AccountsDb {
     /// After this function the number of alive roots is <= # alive roots when it was called.
     /// In practice, the # of alive roots after will be significantly less than # alive roots when called.
     /// Trying to reduce # roots and storages (one per root) required to store all the data in ancient slots
-    pub(crate) fn combine_ancient_slots_packed(
-        &self,
-        sorted_slots: Vec<Slot>,
-        can_randomly_shrink: bool,
-    ) {
+    pub(crate) fn combine_ancient_slots_packed(&self, slots: Vec<Slot>, can_randomly_shrink: bool) {
         let tuning = PackedAncientStorageTuning {
             // only allow 10k slots old enough to be ancient
             max_ancient_slots: 10_000,
@@ -286,11 +285,8 @@ impl AccountsDb {
 
         let mut stats_sub = ShrinkStatsSub::default();
 
-        let (_, total_us) = measure_us!(self.combine_ancient_slots_packed_internal(
-            sorted_slots,
-            tuning,
-            &mut stats_sub
-        ));
+        let (_, total_us) =
+            measure_us!(self.combine_ancient_slots_packed_internal(slots, tuning, &mut stats_sub));
 
         Self::update_shrink_stats(&self.shrink_ancient_stats.shrink_stats, stats_sub, false);
         self.shrink_ancient_stats
@@ -333,14 +329,14 @@ impl AccountsDb {
 
     fn combine_ancient_slots_packed_internal(
         &self,
-        sorted_slots: Vec<Slot>,
+        slots: Vec<Slot>,
         tuning: PackedAncientStorageTuning,
         metrics: &mut ShrinkStatsSub,
     ) {
         self.shrink_ancient_stats
             .slots_considered
-            .fetch_add(sorted_slots.len() as u64, Ordering::Relaxed);
-        let ancient_slot_infos = self.collect_sort_filter_ancient_slots(sorted_slots, &tuning);
+            .fetch_add(slots.len() as u64, Ordering::Relaxed);
+        let ancient_slot_infos = self.collect_sort_filter_ancient_slots(slots, &tuning);
 
         if ancient_slot_infos.all_infos.is_empty() {
             return; // nothing to do