From 07fa00ef396d26e3c554200116eda9098d8f56cf Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:51:19 +0100 Subject: [PATCH] perf: Use a fixed partition count in PartitionedUnorderedMap The default partition count was the host's core count, and TaggedCache::sweep() starts one thread per partition, so a sweep created as many threads as the host has cores. Fix the default at 2, exposed as the kDefaultPartitions constant. Both arms of the constructor's initializer are now non-zero, so the non-zero assertion can no longer fire and is removed. The header no longer needs or instrumentation.h. Add a GTest suite covering the default, the zero fallback, an explicit count, and the key split across partitions. --- .../xrpl/basics/partitioned_unordered_map.h | 33 ++++++--- .../basics/partitioned_unordered_map.cpp | 69 +++++++++++++++++++ 2 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 src/tests/libxrpl/basics/partitioned_unordered_map.cpp diff --git a/include/xrpl/basics/partitioned_unordered_map.h b/include/xrpl/basics/partitioned_unordered_map.h index c6b0107b93..3f3d0e4d19 100644 --- a/include/xrpl/basics/partitioned_unordered_map.h +++ b/include/xrpl/basics/partitioned_unordered_map.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include @@ -9,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -38,9 +36,22 @@ template < typename Alloc = std::allocator>> class PartitionedUnorderedMap { + /** + * How many sub-maps the keys are spread over. Never zero. + */ std::size_t partitions_; public: + /** + * Partition count used when the caller does not ask for one. + * + * A partition is the unit of parallel work for callers that walk the whole + * map: TaggedCache::sweep() runs one thread per partition. Holding this + * small and fixed keeps that thread count independent of how many cores + * the host has. + */ + static constexpr std::size_t kDefaultPartitions = 2; + using key_type = Key; using mapped_type = Value; using value_type = std::pair; @@ -215,19 +226,21 @@ private: } public: + /** + * Builds an empty map spread over a fixed number of partitions. + * + * @param partitions How many partitions to use. An empty optional, or 0, + * selects kDefaultPartitions. + */ PartitionedUnorderedMap(std::optional partitions = std::nullopt) - // Set partitions to the number of hardware threads if the parameter - // is either empty or set to 0. - : partitions_( - partitions && (*partitions != 0u) ? *partitions : std::thread::hardware_concurrency()) + : partitions_(partitions && (*partitions != 0u) ? *partitions : kDefaultPartitions) { map_.resize(partitions_); - XRPL_ASSERT( - partitions_, - "xrpl::PartitionedUnorderedMap::PartitionedUnorderedMap : " - "nonzero partitions"); } + /** + * Returns how many partitions the keys are spread over. + */ std::size_t partitions() const { diff --git a/src/tests/libxrpl/basics/partitioned_unordered_map.cpp b/src/tests/libxrpl/basics/partitioned_unordered_map.cpp new file mode 100644 index 0000000000..7c82bfcf42 --- /dev/null +++ b/src/tests/libxrpl/basics/partitioned_unordered_map.cpp @@ -0,0 +1,69 @@ +#include + +#include + +#include +#include +#include + +namespace xrpl { + +using TestMap = PartitionedUnorderedMap>; + +TEST(PartitionedUnorderedMapTest, default_partition_count_is_two) +{ + // Callers that walk the whole map run one thread per partition, so this + // count bounds that thread count. It must not follow the core count. + EXPECT_EQ(TestMap::kDefaultPartitions, 2u); + + TestMap const map; + EXPECT_EQ(map.partitions(), TestMap::kDefaultPartitions); +} + +TEST(PartitionedUnorderedMapTest, zero_partitions_falls_back_to_the_default) +{ + TestMap const map{0}; + EXPECT_EQ(map.partitions(), TestMap::kDefaultPartitions); +} + +TEST(PartitionedUnorderedMapTest, explicit_partition_count_is_honoured) +{ + TestMap const map{TestMap::kDefaultPartitions + 5}; + EXPECT_EQ(map.partitions(), TestMap::kDefaultPartitions + 5); +} + +TEST(PartitionedUnorderedMapTest, every_key_is_reachable_across_partitions) +{ + // The partitioner is key % partitions, so with two partitions the even + // keys land in one sub-map and the odd keys in the other. + std::size_t const count = 10; + TestMap map; + + for (std::size_t key = 0; key < count; ++key) + map.emplace(key, std::to_string(key)); + + EXPECT_EQ(map.size(), count); + + // Pin the split itself, not just the total: a partitioner that sent every + // key to one sub-map would still satisfy size() and the lookups below. + ASSERT_EQ(map.map().size(), 2u); + EXPECT_EQ(map.map()[0].size(), count / 2); + EXPECT_EQ(map.map()[1].size(), count / 2); + + for (std::size_t key = 0; key < count; ++key) + { + auto const it = map.find(key); + ASSERT_NE(it, map.end()); + EXPECT_EQ(it->second, std::to_string(key)); + } + + std::size_t visited = 0; + for (auto const& entry : map) + { + EXPECT_EQ(entry.second, std::to_string(entry.first)); + ++visited; + } + EXPECT_EQ(visited, count); +} + +} // namespace xrpl