From ea33be049480040e5ce75962e3cc464813157f29 Mon Sep 17 00:00:00 2001
From: bneradt <bneradt@verizonmedia.com>
Date: Tue, 6 Oct 2020 16:13:56 +0000
Subject: [PATCH] RolledLogDeleter: do not sort on each candidate
 consideration.

A performance issue was noticed in Docs testing related to the
RolledLogDeleter candidates consideration. This fixes the candidate
consideration logic to not sort on consideration of every candidate but
rather sort after all the candidates have been gathered (if deletion
will indeed take place).
---
 proxy/logging/RolledLogDeleter.cc | 22 +++++++++++++++++-----
 proxy/logging/RolledLogDeleter.h  | 17 +++++++++++++++--
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/proxy/logging/RolledLogDeleter.cc b/proxy/logging/RolledLogDeleter.cc
index 1547b50fb25..6c8a6e207c9 100644
--- a/proxy/logging/RolledLogDeleter.cc
+++ b/proxy/logging/RolledLogDeleter.cc
@@ -73,6 +73,7 @@ RolledLogDeleter::register_log_type_for_deletion(std::string_view log_type, int
 
   deletingInfoList.push_back(std::move(deletingInfo));
   deleting_info.insert(deletingInfoPtr);
+  candidates_require_sorting = true;
 }
 
 bool
@@ -86,20 +87,31 @@ RolledLogDeleter::consider_for_candidacy(std::string_view log_path, int64_t file
   auto &candidates = iter->candidates;
   candidates.push_back(std::make_unique<LogDeleteCandidate>(log_path, file_size, modification_time));
   ++num_candidates;
-
-  std::sort(
-    candidates.begin(), candidates.end(),
-    [](std::unique_ptr<LogDeleteCandidate> const &a, std::unique_ptr<LogDeleteCandidate> const &b) { return a->mtime > b->mtime; });
-
+  candidates_require_sorting = true;
   return true;
 }
 
+void
+RolledLogDeleter::sort_candidates()
+{
+  deleting_info.apply([](LogDeletingInfo &info) {
+    std::sort(info.candidates.begin(), info.candidates.end(),
+              [](std::unique_ptr<LogDeleteCandidate> const &a, std::unique_ptr<LogDeleteCandidate> const &b) {
+                return a->mtime > b->mtime;
+              });
+  });
+  candidates_require_sorting = false;
+}
+
 std::unique_ptr<LogDeleteCandidate>
 RolledLogDeleter::take_next_candidate_to_delete()
 {
   if (!has_candidates()) {
     return nullptr;
   }
+  if (candidates_require_sorting) {
+    sort_candidates();
+  }
   // Select the highest priority type (diags.log, traffic.out, etc.) from which
   // to select a candidate.
   auto target_type =
diff --git a/proxy/logging/RolledLogDeleter.h b/proxy/logging/RolledLogDeleter.h
index f03785f6e44..4b607f20a22 100644
--- a/proxy/logging/RolledLogDeleter.h
+++ b/proxy/logging/RolledLogDeleter.h
@@ -24,7 +24,7 @@
 #pragma once
 
 #include <cstdint>
-#include <list>
+#include <deque>
 #include <memory>
 #include <string>
 #include <string_view>
@@ -201,14 +201,27 @@ class RolledLogDeleter
    */
   void clear_candidates();
 
+private:
+  /** Sort all the assembled candidates for each LogDeletingInfo.
+   *
+   * After any additions to the @a deleting_info, this should be called before
+   * calling @a take_next_candidate_to_delete because the latter depends upon
+   * the candidate entries being sorted.
+   */
+  void sort_candidates();
+
 private:
   /** The owning references to the set of LogDeletingInfo added to the below
    * hash map. */
-  std::list<std::unique_ptr<LogDeletingInfo>> deletingInfoList;
+  std::deque<std::unique_ptr<LogDeletingInfo>> deletingInfoList;
 
   /** The set of candidates for deletion keyed by log_type. */
   IntrusiveHashMap<LogDeletingInfoDescriptor> deleting_info;
 
   /** The number of tracked candidates. */
   size_t num_candidates = 0;
+
+  /** Whether the candidates require sorting due to an addition to the
+   * deleting_info. */
+  bool candidates_require_sorting = true;
 };