compaction_manager: use unordered_set for compacting sstables registr…

…ation It is more efficient than using a vector as the interface. Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
nawenqiang · Feb 28, 2022 · c008fb1 · c008fb1
1 parent 132c9d5
commit c008fb1
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 21 deletions.
diff --git a/compaction/compaction_manager.cc b/compaction/compaction_manager.cc
@@ -26,13 +26,12 @@ using namespace std::chrono_literals;
 
 class compacting_sstable_registration {
     compaction_manager* _cm;
-    std::vector<sstables::shared_sstable> _compacting;
+    std::unordered_set<sstables::shared_sstable> _compacting;
 public:
     compacting_sstable_registration(compaction_manager* cm, std::vector<sstables::shared_sstable> compacting)
         : _cm(cm)
-        , _compacting(std::move(compacting))
     {
-        _cm->register_compacting_sstables(_compacting);
+        register_compacting(compacting);
     }
 
     compacting_sstable_registration& operator=(const compacting_sstable_registration&) = delete;
@@ -55,15 +54,21 @@ class compacting_sstable_registration {
 
     ~compacting_sstable_registration() {
         if (_cm) {
-            _cm->deregister_compacting_sstables(_compacting);
+            _cm->deregister_compacting_sstables(_compacting.begin(), _compacting.end());
         }
     }
 
+    void register_compacting(const std::vector<sstables::shared_sstable>& sstables) {
+        _compacting.reserve(_compacting.size() + sstables.size());
+        _compacting.insert(sstables.begin(), sstables.end());
+        _cm->register_compacting_sstables(sstables.begin(), sstables.end());
+    }
+
     // Explicitly release compacting sstables
     void release_compacting(const std::vector<sstables::shared_sstable>& sstables) {
-        _cm->deregister_compacting_sstables(sstables);
-        for (auto& sst : sstables) {
-            _compacting.erase(boost::remove(_compacting, sst), _compacting.end());
+        _cm->deregister_compacting_sstables(sstables.begin(), sstables.end());
+        for (const auto& sst : sstables) {
+            _compacting.erase(sst);
         }
     }
 };
@@ -204,27 +209,25 @@ std::vector<sstables::shared_sstable> compaction_manager::get_candidates(const r
     return candidates;
 }
 
-void compaction_manager::register_compacting_sstables(const std::vector<sstables::shared_sstable>& sstables) {
-    std::unordered_set<sstables::shared_sstable> sstables_to_merge;
-    sstables_to_merge.reserve(sstables.size());
-    for (auto& sst : sstables) {
-        sstables_to_merge.insert(sst);
-    }
-
+template <typename Iterator, typename Sentinel>
+requires std::same_as<Sentinel, Iterator> || std::sentinel_for<Sentinel, Iterator>
+void compaction_manager::register_compacting_sstables(Iterator first, Sentinel last) {
     // make all required allocations in advance to merge
     // so it should not throw
-    _compacting_sstables.reserve(_compacting_sstables.size() + sstables.size());
+    _compacting_sstables.reserve(_compacting_sstables.size() + std::distance(first, last));
     try {
-        _compacting_sstables.merge(sstables_to_merge);
+        _compacting_sstables.insert(first, last);
     } catch (...) {
         cmlog.error("Unexpected error when registering compacting SSTables: {}. Ignored...", std::current_exception());
     }
 }
 
-void compaction_manager::deregister_compacting_sstables(const std::vector<sstables::shared_sstable>& sstables) {
+template <typename Iterator, typename Sentinel>
+requires std::same_as<Sentinel, Iterator> || std::sentinel_for<Sentinel, Iterator>
+void compaction_manager::deregister_compacting_sstables(Iterator first, Sentinel last) {
     // Remove compacted sstables from the set of compacting sstables.
-    for (auto& sst : sstables) {
-        _compacting_sstables.erase(sst);
+    for (; first != last; ++first) {
+        _compacting_sstables.erase(*first);
     }
 }
 

diff --git a/compaction/compaction_manager.hh b/compaction/compaction_manager.hh
@@ -184,8 +184,13 @@ private:
     // Get candidates for compaction strategy, which are all sstables but the ones being compacted.
     std::vector<sstables::shared_sstable> get_candidates(const replica::table& t);
 
-    void register_compacting_sstables(const std::vector<sstables::shared_sstable>& sstables);
-    void deregister_compacting_sstables(const std::vector<sstables::shared_sstable>& sstables);
+    template <typename Iterator, typename Sentinel>
+    requires std::same_as<Sentinel, Iterator> || std::sentinel_for<Sentinel, Iterator>
+    void register_compacting_sstables(Iterator first, Sentinel last);
+
+    template <typename Iterator, typename Sentinel>
+    requires std::same_as<Sentinel, Iterator> || std::sentinel_for<Sentinel, Iterator>
+    void deregister_compacting_sstables(Iterator first, Sentinel last);
 
     // gets the table's compaction state
     // throws std::out_of_range exception if not found.