From 0248fce3200eb7639770c7c74a8c3e73654523e8 Mon Sep 17 00:00:00 2001
From: Alex Owens <alexowens90@gmail.com>
Date: Tue, 16 Apr 2024 14:24:53 +0100
Subject: [PATCH] Add [UN]LIKELY to if statements

---
 cpp/arcticdb/processing/aggregation.hpp | 55 +++++++++++--------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/cpp/arcticdb/processing/aggregation.hpp b/cpp/arcticdb/processing/aggregation.hpp
index 3136ef9430..1823fbf3cc 100644
--- a/cpp/arcticdb/processing/aggregation.hpp
+++ b/cpp/arcticdb/processing/aggregation.hpp
@@ -222,13 +222,12 @@ enum class SortedAggregationOperator {
     COUNT
 };
 
-// TODO: Find a way to get rid of the optionals
 template<typename T>
 class SumBucketAggregator {
 public:
     void push(T value) {
         if constexpr (std::is_floating_point_v<T>) {
-            if (!std::isnan(value)) {
+            if (ARCTICDB_LIKELY(!std::isnan(value))) {
                 sum_ = sum_.value_or(0) + value;
             }
         } else {
@@ -237,12 +236,12 @@ class SumBucketAggregator {
     }
 
     T finalize() {
-        auto res = sum_.value_or(static_cast<T>(0));
-        sum_ = std::nullopt;
+        auto res = sum_.value_or(0);
+        sum_.reset();
         return res;
     }
 
-    bool has_value() const {
+    [[nodiscard]] bool has_value() const {
         return sum_.has_value();
     }
 private:
@@ -254,7 +253,7 @@ class MinBucketAggregator {
 public:
     void push(T value) {
         if constexpr (std::is_floating_point_v<T>) {
-            if (!std::isnan(value)) {
+            if (ARCTICDB_LIKELY(!std::isnan(value))) {
                 min_ = std::min(min_.value_or(std::numeric_limits<T>::max()), value);
             }
         } else {
@@ -270,11 +269,11 @@ class MinBucketAggregator {
             debug::check<ErrorCode::E_ASSERTION_FAILURE>(min_.has_value(), "finalize called on non-float MinBucketAggregator with no values");
             res = *min_;
         }
-        min_ = std::nullopt;
+        min_.reset();
         return res;
     }
 
-    bool has_value() const {
+    [[nodiscard]] bool has_value() const {
         return min_.has_value();
     }
 private:
@@ -286,7 +285,7 @@ class MaxBucketAggregator {
 public:
     void push(T value) {
         if constexpr (std::is_floating_point_v<T>) {
-            if (!std::isnan(value)) {
+            if (ARCTICDB_LIKELY(!std::isnan(value))) {
                 max_ = std::max(max_.value_or(std::numeric_limits<T>::lowest()), value);
             }
         } else {
@@ -302,11 +301,11 @@ class MaxBucketAggregator {
             debug::check<ErrorCode::E_ASSERTION_FAILURE>(max_.has_value(), "finalize called on non-float MaxBucketAggregator with no values");
             res = *max_;
         }
-        max_ = std::nullopt;
+        max_.reset();
         return res;
     }
 
-    bool has_value() const {
+    [[nodiscard]] bool has_value() const {
         return max_.has_value();
     }
 private:
@@ -318,7 +317,7 @@ class MeanBucketAggregator {
 public:
     void push(T value) {
         if constexpr (std::is_floating_point_v<T>) {
-            if (!std::isnan(value)) {
+            if (ARCTICDB_LIKELY(!std::isnan(value))) {
                 sum_ = sum_.value_or(0) + value;
                 ++count_;
             }
@@ -330,12 +329,12 @@ class MeanBucketAggregator {
 
     double finalize() {
         auto res = static_cast<double>(sum_.value_or(std::numeric_limits<double>::quiet_NaN())) / static_cast<double>(count_);
-        sum_ = std::nullopt;
+        sum_.reset();
         count_ = 0;
         return res;
     }
 
-    bool has_value() const {
+    [[nodiscard]] bool has_value() const {
         return sum_.has_value();
     }
 private:
@@ -348,15 +347,11 @@ class FirstBucketAggregator {
 public:
     void push(T value) {
         if constexpr (std::is_floating_point_v<T>) {
-            if (first_.has_value()) {
-                if (std::isnan(*first_)) {
-                    first_ = value;
-                }
-            } else {
+            if (ARCTICDB_UNLIKELY(!first_.has_value() || std::isnan(*first_))) {
                 first_ = value;
             }
         } else {
-            if (!first_.has_value()) {
+            if (ARCTICDB_UNLIKELY(!first_.has_value())) {
                 first_ = value;
             }
         }
@@ -370,11 +365,11 @@ class FirstBucketAggregator {
             debug::check<ErrorCode::E_ASSERTION_FAILURE>(first_.has_value(), "finalize called on non-float FirstBucketAggregator with no values");
             res = *first_;
         }
-        first_ = std::nullopt;
+        first_.reset();
         return res;
     }
 
-    bool has_value() const {
+    [[nodiscard]] bool has_value() const {
         return first_.has_value();
     }
 private:
@@ -386,11 +381,7 @@ class LastBucketAggregator {
 public:
     void push(T value) {
         if constexpr (std::is_floating_point_v<T>) {
-            if (last_.has_value()) {
-                if (!std::isnan(value)) {
-                    last_ = value;
-                }
-            } else {
+            if (ARCTICDB_LIKELY(!last_.has_value() || !std::isnan(value))) {
                 last_ = value;
             }
         } else {
@@ -406,11 +397,11 @@ class LastBucketAggregator {
             debug::check<ErrorCode::E_ASSERTION_FAILURE>(last_.has_value(), "finalize called on non-float LastBucketAggregator with no values");
             res = *last_;
         }
-        last_ = std::nullopt;
+        last_.reset();
         return res;
     }
 
-    bool has_value() const {
+    [[nodiscard]] bool has_value() const {
         return last_.has_value();
     }
 private:
@@ -422,7 +413,7 @@ class CountBucketAggregator {
     template<typename T>
     void push(T value) {
         if constexpr (std::is_floating_point_v<T>) {
-            if (!std::isnan(value)) {
+            if (ARCTICDB_LIKELY(!std::isnan(value))) {
                 count_ = count_.value_or(0) + 1;
             }
         } else {
@@ -432,11 +423,11 @@ class CountBucketAggregator {
 
     uint64_t finalize() {
         auto res = count_.value_or(0);
-        count_ = std::nullopt;
+        count_.reset();
         return res;
     }
 
-    bool has_value() const {
+    [[nodiscard]] bool has_value() const {
         return count_.has_value();
     }
 private: