Merge pull request #28085 from vespa-engine/balder/use-single-level-hashmap

Balder/use single level hashmap

3 files changed, 54 insertions, 64 deletions

diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
index b686c6bc80c..243b3c5ecb2 100644
--- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
@@ -604,11 +604,9 @@ PerNodeBucketSpacesStats
 toBucketSpacesStats(const NodeMaintenanceStatsTracker &maintenanceStats)
 {
     PerNodeBucketSpacesStats result;
-    for (const auto &nodeEntry : maintenanceStats.perNodeStats()) {
-        for (const auto &bucketSpaceEntry : nodeEntry.second) {
-            auto bucketSpace = document::FixedBucketSpaces::to_string(bucketSpaceEntry.first);
-            result[nodeEntry.first][bucketSpace] = toBucketSpaceStats(bucketSpaceEntry.second);
-        }
+    for (const auto &entry : maintenanceStats.perNodeStats()) {
+        auto bucketSpace = document::FixedBucketSpaces::to_string(entry.first.bucketSpace());
+        result[entry.first.node()][bucketSpace] = toBucketSpaceStats(entry.second);
     }
     return result;
 }
diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp
index 592d92940d6..b10f5abd0f1 100644
--- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp
+++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp
@@ -9,60 +9,20 @@ namespace storage::distributor {
 
 const NodeMaintenanceStats NodeMaintenanceStatsTracker::_emptyNodeMaintenanceStats;
 
-namespace {
-
-void
-merge_bucket_spaces_stats(NodeMaintenanceStatsTracker::BucketSpacesStats& dest,
-                          const NodeMaintenanceStatsTracker::BucketSpacesStats& src)
-{
-    for (const auto& entry : src) {
-        auto bucket_space = entry.first;
-        dest[bucket_space].merge(entry.second);
-    }
-}
-
-}
-
-void
-NodeMaintenanceStatsTracker::incMovingOut(uint16_t node, document::BucketSpace bucketSpace) {
-    ++_node_stats[node][bucketSpace].movingOut;
-    ++_total_stats.movingOut;
-}
-
-void
-NodeMaintenanceStatsTracker::incSyncing(uint16_t node, document::BucketSpace bucketSpace) {
-    ++_node_stats[node][bucketSpace].syncing;
-    ++_total_stats.syncing;
-}
-
-void
-NodeMaintenanceStatsTracker::incCopyingIn(uint16_t node, document::BucketSpace bucketSpace) {
-    ++_node_stats[node][bucketSpace].copyingIn;
-    ++_total_stats.copyingIn;
+NodeMaintenanceStats &
+NodeMaintenanceStatsTracker::stats(uint16_t node, document::BucketSpace bucketSpace) {
+    return _node_stats[BucketSpaceAndNode(node, bucketSpace)];
 }
 
-void
-NodeMaintenanceStatsTracker::incCopyingOut(uint16_t node, document::BucketSpace bucketSpace) {
-    ++_node_stats[node][bucketSpace].copyingOut;
-    ++_total_stats.copyingOut;
-}
-
-void
-NodeMaintenanceStatsTracker::NodeMaintenanceStatsTracker::incTotal(uint16_t node, document::BucketSpace bucketSpace) {
-    ++_node_stats[node][bucketSpace].total;
-    ++_total_stats.total;
+const NodeMaintenanceStats &
+NodeMaintenanceStatsTracker::stats(uint16_t node, document::BucketSpace bucketSpace) const noexcept {
+    auto nodeItr = _node_stats.find(BucketSpaceAndNode(node, bucketSpace));
+    return (nodeItr != _node_stats.end()) ? nodeItr->second : _emptyNodeMaintenanceStats;
 }
 
 const NodeMaintenanceStats&
-NodeMaintenanceStatsTracker::forNode(uint16_t node, document::BucketSpace bucketSpace) const {
-    auto nodeItr = _node_stats.find(node);
-    if (nodeItr != _node_stats.end()) {
-        auto bucketSpaceItr = nodeItr->second.find(bucketSpace);
-        if (bucketSpaceItr != nodeItr->second.end()) {
-            return bucketSpaceItr->second;
-        }
-    }
-    return _emptyNodeMaintenanceStats;
+NodeMaintenanceStatsTracker::forNode(uint16_t node, document::BucketSpace bucketSpace) const noexcept {
+    return stats(node, bucketSpace);
 }
 
 bool
@@ -75,8 +35,8 @@ void
 NodeMaintenanceStatsTracker::merge(const NodeMaintenanceStatsTracker& rhs)
 {
     for (const auto& entry : rhs._node_stats) {
-        auto node_index = entry.first;
-        merge_bucket_spaces_stats(_node_stats[node_index], entry.second);
+        auto key = entry.first;
+        _node_stats[key].merge(entry.second);
     }
     _max_observed_time_since_last_gc = std::max(_max_observed_time_since_last_gc,
                                                 rhs._max_observed_time_since_last_gc);
diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h
index 84705fbca9d..a5cb12de9a4 100644
--- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h
+++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h
@@ -51,8 +51,23 @@ std::ostream& operator<<(std::ostream&, const NodeMaintenanceStats&);
 class NodeMaintenanceStatsTracker
 {
 public:
-    using BucketSpacesStats = vespalib::hash_map<document::BucketSpace, NodeMaintenanceStats, document::BucketSpace::hash>;
-    using PerNodeStats = vespalib::hash_map<uint16_t, BucketSpacesStats>;
+    class BucketSpaceAndNode {
+    public:
+        BucketSpaceAndNode(uint16_t node_in, document::BucketSpace bucketSpace_in) noexcept
+            : _bucketSpace(bucketSpace_in),
+              _node(node_in)
+        {}
+        uint32_t hash() const noexcept { return (uint32_t(_node) << 2) | (_bucketSpace.getId() & 0x3); }
+        bool operator == (const BucketSpaceAndNode & b) const noexcept {
+            return (_bucketSpace == b._bucketSpace) && (_node == b._node);
+        }
+        document::BucketSpace bucketSpace() const noexcept { return _bucketSpace; }
+        uint16_t node() const noexcept { return _node; }
+    private:
+        document::BucketSpace _bucketSpace;
+        uint16_t              _node;
+    };
+    using PerNodeStats = vespalib::hash_map<BucketSpaceAndNode, NodeMaintenanceStats>;
 
 private:
     PerNodeStats         _node_stats;
@@ -61,6 +76,8 @@ private:
 
     static const NodeMaintenanceStats _emptyNodeMaintenanceStats;
 
+    NodeMaintenanceStats & stats(uint16_t node, document::BucketSpace bucketSpace);
+    const NodeMaintenanceStats & stats(uint16_t node, document::BucketSpace bucketSpace) const noexcept;
 public:
     NodeMaintenanceStatsTracker() noexcept;
     NodeMaintenanceStatsTracker(NodeMaintenanceStatsTracker &&) noexcept;
@@ -70,15 +87,30 @@ public:
     void reset(size_t nodes);
     size_t numNodes() const { return _node_stats.size(); }
 
-    void incMovingOut(uint16_t node, document::BucketSpace bucketSpace);
+    void incMovingOut(uint16_t node, document::BucketSpace bucketSpace) {
+        ++stats(node, bucketSpace).movingOut;
+        ++_total_stats.movingOut;
+    }
 
-    void incSyncing(uint16_t node, document::BucketSpace bucketSpace);
+    void incSyncing(uint16_t node, document::BucketSpace bucketSpace) {
+        ++stats(node, bucketSpace).syncing;
+        ++_total_stats.syncing;
+    }
 
-    void incCopyingIn(uint16_t node, document::BucketSpace bucketSpace);
+    void incCopyingIn(uint16_t node, document::BucketSpace bucketSpace) {
+        ++stats(node, bucketSpace).copyingIn;
+        ++_total_stats.copyingIn;
+    }
 
-    void incCopyingOut(uint16_t node, document::BucketSpace bucketSpace);
+    void incCopyingOut(uint16_t node, document::BucketSpace bucketSpace) {
+        ++stats(node, bucketSpace).copyingOut;
+        ++_total_stats.copyingOut;
+    }
 
-    void incTotal(uint16_t node, document::BucketSpace bucketSpace);
+    void incTotal(uint16_t node, document::BucketSpace bucketSpace) {
+        ++stats(node, bucketSpace).total;
+        ++_total_stats.total;
+    }
 
     void update_observed_time_since_last_gc(vespalib::duration time_since_gc) noexcept {
         _max_observed_time_since_last_gc = std::max(time_since_gc, _max_observed_time_since_last_gc);
@@ -88,7 +120,7 @@ public:
      * Returned statistics for a given node index and bucket space, or all zero statistics
      * if none have been recorded yet
      */
-    const NodeMaintenanceStats& forNode(uint16_t node, document::BucketSpace bucketSpace) const;
+    const NodeMaintenanceStats& forNode(uint16_t node, document::BucketSpace bucketSpace) const noexcept;
 
     const PerNodeStats& perNodeStats() const noexcept {
         return _node_stats;