Merge pull request #12997 from vespa-engine/vekterli/constexpr-ifs-for-most-aggregation-call-sites

Make most hasAggregated checks constexpr

8 files changed, 47 insertions, 50 deletions

diff --git a/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp b/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp
index fb912499c6c..2cd7d80e06c 100644
--- a/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp
@@ -93,14 +93,14 @@ normalize()
             _leaf = LeafNodeTypeRefPair(NodeRef(), static_cast<LeafNodeType *>(nullptr));
 
         }
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             Aggregator::recalc(*leafNode, _aggrCalc);
         }
         assert(_numInserts == leafNode->validSlots());
         return;
     }
 
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         Aggregator::recalc(*leafNode, _aggrCalc);
     }
     /* Adjust validLeaves for rightmost nodes */
@@ -115,7 +115,7 @@ normalize()
                       _allocator.mapLeafRef(lcRef)->getLastKey() :
                       _allocator.mapInternalRef(lcRef)->getLastKey(),
                       lcRef);
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             Aggregator::recalc(*inode, _allocator, _aggrCalc);
         }
     }
@@ -166,12 +166,12 @@ normalize()
             _allocator.holdNode(_leaf.ref, leafNode);
             _numLeafNodes--;
             _leaf = LeafNodeTypeRefPair(child, leftLeaf);
-            if (AggrCalcT::hasAggregated()) {
+            if constexpr (AggrCalcT::hasAggregated()) {
                 Aggregator::recalc(*leftLeaf, _aggrCalc);
             }
         } else {
             leafNode->stealSomeFromLeftNode(leftLeaf);
-            if (AggrCalcT::hasAggregated()) {
+            if constexpr (AggrCalcT::hasAggregated()) {
                 Aggregator::recalc(*leftLeaf, _aggrCalc);
                 Aggregator::recalc(*leafNode, _aggrCalc);
             }
@@ -182,7 +182,7 @@ normalize()
                                  pnode->validLeaves();
                 pnode->incValidLeaves(steal);
                 lpnode->decValidLeaves(steal);
-                if (AggrCalcT::hasAggregated()) {
+                if constexpr (AggrCalcT::hasAggregated()) {
                     Aggregator::recalc(*lpnode, _allocator, _aggrCalc);
                     Aggregator::recalc(*pnode, _allocator, _aggrCalc);
                 }
@@ -230,12 +230,12 @@ normalize()
                 _allocator.holdNode(_inodes[level].ref, inode);
                 _numInternalNodes--;
                 _inodes[level] = InternalNodeTypeRefPair(leftInodeRef, leftInode);
-                if (AggrCalcT::hasAggregated()) {
+                if constexpr (AggrCalcT::hasAggregated()) {
                     Aggregator::recalc(*leftInode, _allocator, _aggrCalc);
                 }
             } else {
                 inode->stealSomeFromLeftNode(leftInode, _allocator);
-                if (AggrCalcT::hasAggregated()) {
+                if constexpr (AggrCalcT::hasAggregated()) {
                     Aggregator::recalc(*leftInode, _allocator, _aggrCalc);
                     Aggregator::recalc(*inode, _allocator, _aggrCalc);
                 }
@@ -246,7 +246,7 @@ normalize()
                                      pnode->validLeaves();
                     pnode->incValidLeaves(steal);
                     lpnode->decValidLeaves(steal);
-                    if (AggrCalcT::hasAggregated()) {
+                    if constexpr (AggrCalcT::hasAggregated()) {
                         Aggregator::recalc(*lpnode, _allocator, _aggrCalc);
                         Aggregator::recalc(*pnode, _allocator, _aggrCalc);
                     }
@@ -303,7 +303,7 @@ allocNewLeafNode()
     InternalNodeType  *inode;
     NodeRef child;
 
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         Aggregator::recalc(*_leaf.data, _aggrCalc);
     }
     LeafNodeTypeRefPair lPair(_allocator.allocLeafNode());
diff --git a/vespalib/src/vespa/vespalib/btree/btreeinserter.hpp b/vespalib/src/vespa/vespalib/btree/btreeinserter.hpp
index b24874088a2..e1518da5639 100644
--- a/vespalib/src/vespa/vespalib/btree/btreeinserter.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreeinserter.hpp
@@ -29,6 +29,7 @@ template <typename KeyT, typename DataT, typename AggrT, typename CompareT,
 void
 BTreeInserter<KeyT, DataT, AggrT, CompareT, TraitsT, AggrCalcT>::rebalanceLeafEntries(LeafNodeType *leafNode, Iterator &itr, AggrCalcT aggrCalc)
 {
+    (void)aggrCalc;
     NodeAllocatorType &allocator(itr.getAllocator());
     auto &pathElem = itr.getPath(0);
     InternalNodeType *parentNode = pathElem.getWNode();
@@ -58,7 +59,7 @@ BTreeInserter<KeyT, DataT, AggrT, CompareT, TraitsT, AggrCalcT>::rebalanceLeafEn
             uint32_t given = leftNode->validSlots() - oldLeftValid;
             parentNode->update(parentIdx, leafNode->getLastKey(), leafRef);
             parentNode->update(parentIdx - 1, leftNode->getLastKey(), leftRef);
-            if (AggrCalcT::hasAggregated()) {
+            if constexpr (AggrCalcT::hasAggregated()) {
                 Aggregator::recalc(*leftNode, allocator, aggrCalc);
                 Aggregator::recalc(*leafNode, allocator, aggrCalc);
             }
@@ -69,7 +70,7 @@ BTreeInserter<KeyT, DataT, AggrT, CompareT, TraitsT, AggrCalcT>::rebalanceLeafEn
         rightNode->stealSomeFromLeftNode(leafNode, allocator);
         parentNode->update(parentIdx, leafNode->getLastKey(), leafRef);
         parentNode->update(parentIdx + 1, rightNode->getLastKey(), rightRef);
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             Aggregator::recalc(*rightNode, allocator, aggrCalc);
             Aggregator::recalc(*leafNode, allocator, aggrCalc);
         }
@@ -110,7 +111,7 @@ insert(BTreeNode::Ref &root,
     if (lnode->isFull()) {
         LeafNodeTypeRefPair splitNode = allocator.allocLeafNode();
         lnode->splitInsert(splitNode.data, idx, key, data);
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             ca = Aggregator::recalc(*lnode, *splitNode.data, aggrCalc);
         }
         splitNodeRef = splitNode.ref; // to signal that a split occured
@@ -119,7 +120,7 @@ insert(BTreeNode::Ref &root,
     } else {
         lnode->insert(idx, key, data);
         itr.setLeafNodeIdx(idx);
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             aggrCalc.add(lnode->getAggregated(), aggrCalc.getVal(data));
             ca = lnode->getAggregated();
         }
@@ -144,7 +145,7 @@ insert(BTreeNode::Ref &root,
                 node->splitInsert(splitNode.data, idx,
                                   *splitLastKey, splitNodeRef, allocator);
                 inRightSplit = pe.adjustSplit(inRightSplit, splitNode.data);
-                if (AggrCalcT::hasAggregated()) {
+                if constexpr (AggrCalcT::hasAggregated()) {
                     ca = Aggregator::recalc(*node, *splitNode.data,
                                             allocator, aggrCalc);
                 }
@@ -154,7 +155,7 @@ insert(BTreeNode::Ref &root,
                 node->insert(idx, *splitLastKey, splitNodeRef);
                 pe.adjustSplit(inRightSplit);
                 inRightSplit = false;
-                if (AggrCalcT::hasAggregated()) {
+                if constexpr (AggrCalcT::hasAggregated()) {
                     aggrCalc.add(node->getAggregated(), oldca, ca);
                     ca = node->getAggregated();
                 }
@@ -162,12 +163,12 @@ insert(BTreeNode::Ref &root,
                 splitLastKey = nullptr;
             }
         } else {
-            if (AggrCalcT::hasAggregated()) {
+            if constexpr (AggrCalcT::hasAggregated()) {
                 aggrCalc.add(node->getAggregated(), oldca, ca);
                 ca = node->getAggregated();
             }
         }
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             oldca = olda;
         }
         lastKey = &node->getLastKey();
diff --git a/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp b/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp
index 13d41ef61f3..27a50e04f1b 100644
--- a/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp
@@ -427,7 +427,7 @@ BTreeIteratorBase<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS, PATH_SIZE>::
 BTreeIteratorBase(const KeyDataType *shortArray,
                   uint32_t arraySize,
                   const NodeAllocatorType &allocator,
-                  const AggrCalcT &aggrCalc)
+                  [[maybe_unused]] const AggrCalcT &aggrCalc)
     : _leaf(nullptr, 0u),
       _path(),
       _pathSize(0),
@@ -439,11 +439,9 @@ BTreeIteratorBase(const KeyDataType *shortArray,
         _compatLeafNode.reset(new LeafNodeTempType(shortArray, arraySize));
         _leaf.setNode(_compatLeafNode.get());
         _leafRoot = _leaf.getNode();
-        typedef BTreeAggregator<KeyT, DataT, AggrT,
-            INTERNAL_SLOTS, LEAF_SLOTS, AggrCalcT> Aggregator;
-        if (AggrCalcT::hasAggregated()) {
-            Aggregator::recalc(const_cast<LeafNodeType &>(*_leaf.getNode()),
-                               aggrCalc);
+        if constexpr (AggrCalcT::hasAggregated()) {
+            using Aggregator = BTreeAggregator<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS, AggrCalcT>;
+            Aggregator::recalc(const_cast<LeafNodeType &>(*_leaf.getNode()), aggrCalc);
         }
     }
 }
@@ -1088,10 +1086,10 @@ template <typename KeyT, typename DataT, typename AggrT, typename CompareT,
 template <class AggrCalcT>
 void
 BTreeIterator<KeyT, DataT, AggrT, CompareT, TraitsT>::
-updateData(const DataType & data, const AggrCalcT &aggrCalc)
+updateData(const DataType & data, [[maybe_unused]] const AggrCalcT &aggrCalc)
 {
     LeafNodeType * lnode = getLeafNode();
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         AggrT oldca(lnode->getAggregated());
         typedef BTreeAggregator<KeyT, DataT, AggrT,
             TraitsT::INTERNAL_SLOTS,
@@ -1186,14 +1184,14 @@ template <class AggrCalcT>
 BTreeNode::Ref
 BTreeIterator<KeyT, DataT, AggrT, CompareT, TraitsT>::
 insertFirst(const KeyType &key, const DataType &data,
-            const AggrCalcT &aggrCalc)
+            [[maybe_unused]] const AggrCalcT &aggrCalc)
 {
     assert(_pathSize == 0);
     assert(_leafRoot == nullptr);
     NodeAllocatorType &allocator = getAllocator();
     LeafNodeTypeRefPair lnode = allocator.allocLeafNode();
     lnode.data->insert(0, key, data);
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         AggrT a;
         aggrCalc.add(a, aggrCalc.getVal(data));
         lnode.data->getAggregated() = a;
@@ -1231,12 +1229,12 @@ template <class AggrCalcT>
 BTreeNode::Ref
 BTreeIterator<KeyT, DataT, AggrT, CompareT, TraitsT>::
 addLevel(BTreeNode::Ref rootRef, BTreeNode::Ref splitNodeRef,
-         bool inRightSplit, const AggrCalcT &aggrCalc)
+         bool inRightSplit, [[maybe_unused]] const AggrCalcT &aggrCalc)
 {
-    typedef BTreeAggregator<KeyT, DataT, AggrT,
-                            TraitsT::INTERNAL_SLOTS,
-                            TraitsT::LEAF_SLOTS,
-                            AggrCalcT> Aggregator;
+    using Aggregator = BTreeAggregator<KeyT, DataT, AggrT,
+                                       TraitsT::INTERNAL_SLOTS,
+                                       TraitsT::LEAF_SLOTS,
+                                       AggrCalcT>;
 
     NodeAllocatorType &allocator(getAllocator());
     
@@ -1246,7 +1244,7 @@ addLevel(BTreeNode::Ref rootRef, BTreeNode::Ref splitNodeRef,
                           allocator.validLeaves(splitNodeRef));
     inode->insert(0, allocator.getLastKey(rootRef), rootRef);
     inode->insert(1, allocator.getLastKey(splitNodeRef), splitNodeRef);
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         Aggregator::recalc(*inode, allocator, aggrCalc);
     }
     _path[_pathSize].setNodeAndIdx(inode, inRightSplit ? 1u : 0u);
diff --git a/vespalib/src/vespa/vespalib/btree/btreeremover.hpp b/vespalib/src/vespa/vespalib/btree/btreeremover.hpp
index 2281fd99f6d..6a9a7ebcc7e 100644
--- a/vespalib/src/vespa/vespalib/btree/btreeremover.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreeremover.hpp
@@ -16,7 +16,7 @@ BTreeRemoverBase<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS, AggrCalcT>::
 steal(InternalNodeType *pNode,
       BTreeNode::Ref sNodeRef,
       NodeType * sNode, uint32_t idx, NodeAllocatorType &allocator,
-      const AggrCalcT &aggrCalc,
+      [[maybe_unused]] const AggrCalcT &aggrCalc,
       Iterator &itr,
       uint32_t level)
 {
@@ -65,7 +65,7 @@ steal(InternalNodeType *pNode,
         uint32_t stolen = oldLeftValid - leftVictim->validSlots();
         pNode->update(idx, sNode->getLastKey(), sNodeRef);
         pNode->update(idx - 1, leftVictim->getLastKey(), leftVictimRef);
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             Aggregator::recalc(*leftVictim, allocator, aggrCalc);
         }
         itr.adjustSteal(level, false, stolen);
@@ -79,11 +79,11 @@ steal(InternalNodeType *pNode,
         sNode->stealSomeFromRightNode(rightVictim, allocator);
         pNode->update(idx, sNode->getLastKey(), sNodeRef);
         pNode->update(idx + 1, rightVictim->getLastKey(), rightVictimRef);
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             Aggregator::recalc(*rightVictim, allocator, aggrCalc);
         }
     }
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         Aggregator::recalc(*sNode, allocator, aggrCalc);
     }
 }
@@ -118,7 +118,7 @@ remove(BTreeNode::Ref &root,
     } else {
         lnode->remove(idx);
     }
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         ca = lnode->getAggregated();
     }
     bool steppedBack = idx >= lnode->validSlots();
@@ -160,7 +160,7 @@ remove(BTreeNode::Ref &root,
                      itr, level);
             }
         }
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             if (aggrCalc.remove(node->getAggregated(), oldca, ca)) {
                 Aggregator::recalc(*node, allocator, aggrCalc);
             }
diff --git a/vespalib/src/vespa/vespalib/btree/btreeroot.hpp b/vespalib/src/vespa/vespalib/btree/btreeroot.hpp
index 22703f2dfd2..3893e0e0124 100644
--- a/vespalib/src/vespa/vespalib/btree/btreeroot.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreeroot.hpp
@@ -63,7 +63,7 @@ isValid(BTreeNode::Ref node,
                 return false;
             }
         }
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             AggrT aggregated = Aggregator::aggregate(*lnode, aggrCalc);
             if (aggregated != lnode->getAggregated()) {
                 return false;
@@ -113,7 +113,7 @@ isValid(BTreeNode::Ref node,
         if (lChildren < inode->validSlots() && iChildren < inode->validSlots()) {
             return false;
         }
-        if (AggrCalcT::hasAggregated()) {
+        if constexpr (AggrCalcT::hasAggregated()) {
             AggrT aggregated = Aggregator::aggregate(*inode, allocator, aggrCalc);
             if (aggregated != inode->getAggregated()) {
                 return false;
diff --git a/vespalib/src/vespa/vespalib/btree/btreestore.hpp b/vespalib/src/vespa/vespalib/btree/btreestore.hpp
index 614546903dc..ec3be588de4 100644
--- a/vespalib/src/vespa/vespalib/btree/btreestore.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreestore.hpp
@@ -176,7 +176,7 @@ makeTree(EntryRef &ref,
     }
     typedef BTreeAggregator<KeyT, DataT, AggrT,
         TraitsT::INTERNAL_SLOTS, TraitsT::LEAF_SLOTS, AggrCalcT> Aggregator;
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         Aggregator::recalc(*lNode, _aggrCalc);
     }
     lNode->freeze();
@@ -304,7 +304,7 @@ insert(EntryRef &ref,
     assert(idx == clusterSize + 1);
     typedef BTreeAggregator<KeyT, DataT, AggrT,
         TraitsT::INTERNAL_SLOTS, TraitsT::LEAF_SLOTS, AggrCalcT> Aggregator;
-    if (AggrCalcT::hasAggregated()) {
+    if constexpr (AggrCalcT::hasAggregated()) {
         Aggregator::recalc(*lNode, _aggrCalc);
     }
     lNode->freeze();
diff --git a/vespalib/src/vespa/vespalib/btree/minmaxaggrcalc.h b/vespalib/src/vespa/vespalib/btree/minmaxaggrcalc.h
index b33422ec3e3..66e23127c5a 100644
--- a/vespalib/src/vespa/vespalib/btree/minmaxaggrcalc.h
+++ b/vespalib/src/vespa/vespalib/btree/minmaxaggrcalc.h
@@ -9,8 +9,8 @@ namespace search::btree {
 class MinMaxAggrCalc
 {
 public:
-    MinMaxAggrCalc() { }
-    static bool hasAggregated() { return true; }
+    constexpr MinMaxAggrCalc() = default;
+    constexpr static bool hasAggregated() { return true; }
     static int32_t getVal(int32_t val) { return val; }
     static void add(MinMaxAggregated &a, int32_t val) { a.add(val); }
     static void add(MinMaxAggregated &a, const MinMaxAggregated &ca) { a.add(ca); }
diff --git a/vespalib/src/vespa/vespalib/btree/noaggrcalc.h b/vespalib/src/vespa/vespalib/btree/noaggrcalc.h
index e77e8bc204a..079abc57224 100644
--- a/vespalib/src/vespa/vespalib/btree/noaggrcalc.h
+++ b/vespalib/src/vespa/vespalib/btree/noaggrcalc.h
@@ -9,11 +9,9 @@ namespace search::btree {
 class NoAggrCalc
 {
 public:
-    NoAggrCalc()
-    {
-    }
+    constexpr NoAggrCalc() = default;
 
-    static bool
+    constexpr static bool
     hasAggregated()
     {
         return false;