Use hash maps wherever possible (#289)

* Implement legate::hash and change some of the caches to unordered_map

* Use hash maps wherever applicable

* Remove unused parameter

* Fix for clang-tidy errors

* Address review comments

* Make the use of custom hashers explicit in definitions

* Address review comments and turn some of the hashers to std::hash

* Minor clean-up

Author: magnatelee

legate_core_cpp.cmake

@@ -565,6 +565,7 @@ install(
   FILES src/core/utilities/debug.h
         src/core/utilities/debug.inl
         src/core/utilities/dispatch.h
+        src/core/utilities/hash.h
         src/core/utilities/machine.h
         src/core/utilities/memory.h
         src/core/utilities/memory.inl

src/core/data/detail/logical_array.cc

@@ -141,7 +141,7 @@ void BaseLogicalArray::record_scalar_reductions(AutoTask* task, Legion::Reductio
 
 void BaseLogicalArray::generate_constraints(
   AutoTask* task,
-  std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+  std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
   const Variable* partition_symbol) const
 {
   mapping.try_emplace(data_, partition_symbol);
@@ -155,7 +155,7 @@ void BaseLogicalArray::generate_constraints(
 }
 
 std::unique_ptr<Analyzable> BaseLogicalArray::to_launcher_arg(
-  const std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+  const std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
   const Strategy& strategy,
   const Domain& launch_domain,
   const std::optional<SymbolicPoint>& projection,
@@ -273,7 +273,7 @@ void ListLogicalArray::record_scalar_reductions(AutoTask* task, Legion::Reductio
 
 void ListLogicalArray::generate_constraints(
   AutoTask* task,
-  std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+  std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
   const Variable* partition_symbol) const
 {
   descriptor_->generate_constraints(task, mapping, partition_symbol);
@@ -285,7 +285,7 @@ void ListLogicalArray::generate_constraints(
 }
 
 std::unique_ptr<Analyzable> ListLogicalArray::to_launcher_arg(
-  const std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+  const std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
   const Strategy& strategy,
   const Domain& launch_domain,
   const std::optional<SymbolicPoint>& projection,
@@ -452,7 +452,7 @@ void StructLogicalArray::record_scalar_reductions(AutoTask* task, Legion::Reduct
 
 void StructLogicalArray::generate_constraints(
   AutoTask* task,
-  std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+  std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
   const Variable* partition_symbol) const
 {
   auto it = fields_.begin();
@@ -472,7 +472,7 @@ void StructLogicalArray::generate_constraints(
 }
 
 std::unique_ptr<Analyzable> StructLogicalArray::to_launcher_arg(
-  const std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+  const std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
   const Strategy& strategy,
   const Domain& launch_domain,
   const std::optional<SymbolicPoint>& projection,

src/core/data/detail/logical_array.h

@@ -20,6 +20,7 @@
 #include <map>
 #include <memory>
 #include <optional>
+#include <unordered_map>
 #include <vector>
 
 namespace legate::detail {
@@ -68,11 +69,11 @@ struct LogicalArray {
 
   virtual void generate_constraints(
     AutoTask* task,
-    std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
     const Variable* partition_symbol) const = 0;
 
   [[nodiscard]] virtual std::unique_ptr<Analyzable> to_launcher_arg(
-    const std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    const std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
     const Strategy& strategy,
     const Domain& launch_domain,
     const std::optional<SymbolicPoint>& projection,
@@ -119,12 +120,13 @@ class BaseLogicalArray final : public LogicalArray {
   void record_scalar_or_unbound_outputs(AutoTask* task) const override;
   void record_scalar_reductions(AutoTask* task, Legion::ReductionOpID redop) const override;
 
-  void generate_constraints(AutoTask* task,
-                            std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
-                            const Variable* partition_symbol) const override;
+  void generate_constraints(
+    AutoTask* task,
+    std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    const Variable* partition_symbol) const override;
 
   [[nodiscard]] std::unique_ptr<Analyzable> to_launcher_arg(
-    const std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    const std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
     const Strategy& strategy,
     const Domain& launch_domain,
     const std::optional<SymbolicPoint>& projection,
@@ -173,12 +175,13 @@ class ListLogicalArray final : public LogicalArray {
   void record_scalar_or_unbound_outputs(AutoTask* task) const override;
   void record_scalar_reductions(AutoTask* task, Legion::ReductionOpID redop) const override;
 
-  void generate_constraints(AutoTask* task,
-                            std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
-                            const Variable* partition_symbol) const override;
+  void generate_constraints(
+    AutoTask* task,
+    std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    const Variable* partition_symbol) const override;
 
   [[nodiscard]] std::unique_ptr<Analyzable> to_launcher_arg(
-    const std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    const std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
     const Strategy& strategy,
     const Domain& launch_domain,
     const std::optional<SymbolicPoint>& projection,
@@ -226,12 +229,13 @@ class StructLogicalArray final : public LogicalArray {
   void record_scalar_or_unbound_outputs(AutoTask* task) const override;
   void record_scalar_reductions(AutoTask* task, Legion::ReductionOpID redop) const override;
 
-  void generate_constraints(AutoTask* task,
-                            std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
-                            const Variable* partition_symbol) const override;
+  void generate_constraints(
+    AutoTask* task,
+    std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    const Variable* partition_symbol) const override;
 
   [[nodiscard]] std::unique_ptr<Analyzable> to_launcher_arg(
-    const std::map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
+    const std::unordered_map<std::shared_ptr<LogicalStore>, const Variable*>& mapping,
     const Strategy& strategy,
     const Domain& launch_domain,
     const std::optional<SymbolicPoint>& projection,

src/core/mapping/detail/base_mapper.cc

@@ -487,8 +487,7 @@ void BaseMapper::map_task(Legion::Mapping::MapperContext ctx,
   map_unbound_stores(for_unbound_stores);
 
   output.chosen_instances.resize(task.regions.size());
-  std::map<const Legion::RegionRequirement*, std::vector<Legion::Mapping::PhysicalInstance>*>
-    output_map;
+  OutputMap output_map;
   for (uint32_t idx = 0; idx < task.regions.size(); ++idx) {
     output_map[&task.regions[idx]] = &output.chosen_instances[idx];
   }
@@ -1066,8 +1065,7 @@ void BaseMapper::map_inline(Legion::Mapping::MapperContext ctx,
 
   mappings.push_back(StoreMapping::default_mapping(&store, store_target, false));
 
-  std::map<const Legion::RegionRequirement*, std::vector<Legion::Mapping::PhysicalInstance>*>
-    output_map;
+  OutputMap output_map;
   auto&& reqs = mappings.front()->requirements();
   for (auto* req : reqs) {
     output_map[req] = &output.chosen_instances;
@@ -1146,8 +1144,7 @@ void BaseMapper::map_copy(Legion::Mapping::MapperContext ctx,
 
   auto store_target = default_store_targets(target_proc.kind()).front();
 
-  std::map<const Legion::RegionRequirement*, std::vector<Legion::Mapping::PhysicalInstance>*>
-    output_map;
+  OutputMap output_map;
   auto add_to_output_map = [&output_map](auto& reqs, auto& instances) {
     instances.resize(reqs.size());
     for (uint32_t idx = 0; idx < reqs.size(); ++idx) {
@@ -1339,8 +1336,7 @@ void BaseMapper::map_partition(Legion::Mapping::MapperContext ctx,
   std::vector<std::unique_ptr<StoreMapping>> mappings;
   mappings.push_back(StoreMapping::default_mapping(&store, store_target, false));
 
-  std::map<const Legion::RegionRequirement*, std::vector<Legion::Mapping::PhysicalInstance>*>
-    output_map;
+  OutputMap output_map;
   auto&& reqs = mappings.front()->requirements();
   for (auto* req : reqs) {
     output_map[req] = &output.chosen_instances;

src/core/mapping/detail/base_mapper.h

@@ -16,15 +16,16 @@
 #include "core/mapping/detail/machine.h"
 #include "core/mapping/detail/mapping.h"
 #include "core/runtime/detail/library.h"
+#include "core/utilities/detail/hash.h"
 #include "core/utilities/typedefs.h"
 
 #include "legion.h"
 
-#include <map>
 #include <memory>
 #include <optional>
 #include <set>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
 namespace legate::mapping::detail {
@@ -246,8 +247,8 @@ class BaseMapper final : public Legion::Mapping::Mapper, public MachineQueryInte
                           const MapperTaskResult& result) override;
 
  protected:
-  using OutputMap =
-    std::map<const Legion::RegionRequirement*, std::vector<Legion::Mapping::PhysicalInstance>*>;
+  using OutputMap = std::unordered_map<const Legion::RegionRequirement*,
+                                       std::vector<Legion::Mapping::PhysicalInstance>*>;
   void map_legate_stores(Legion::Mapping::MapperContext ctx,
                          const Legion::Mappable& mappable,
                          std::vector<std::unique_ptr<StoreMapping>>& mappings,
@@ -296,7 +297,8 @@ class BaseMapper final : public Legion::Mapping::Mapper, public MachineQueryInte
 
  protected:
   using VariantCacheKey = std::pair<Legion::TaskID, Processor::Kind>;
-  std::map<VariantCacheKey, std::optional<Legion::VariantID>> variants{};
+  std::unordered_map<VariantCacheKey, std::optional<Legion::VariantID>, hasher<VariantCacheKey>>
+    variants{};
 
   InstanceManager* local_instances{};
   ReductionInstanceManager* reduction_instances{};

src/core/mapping/detail/instance_manager.cc

@@ -103,9 +103,10 @@ bool too_big(size_t union_volume, size_t my_volume, size_t group_volume, size_t
 
 struct construct_overlapping_region_group_fn {
   template <int32_t DIM>
-  RegionGroupP operator()(const InstanceSet::Region& region,
-                          const Domain& domain,
-                          const std::map<RegionGroup*, InstanceSet::InstanceSpec>& instances)
+  RegionGroupP operator()(
+    const InstanceSet::Region& region,
+    const Domain& domain,
+    const std::unordered_map<RegionGroup*, InstanceSet::InstanceSpec>& instances)
   {
     auto bound       = domain.bounds<DIM, coord_t>();
     size_t bound_vol = bound.volume();

src/core/mapping/detail/instance_manager.h

@@ -13,11 +13,14 @@
 #pragma once
 
 #include "core/mapping/mapping.h"
+#include "core/utilities/detail/hash.h"
+#include "core/utilities/hash.h"
 
 #include <iosfwd>
 #include <map>
 #include <memory>
 #include <set>
+#include <unordered_map>
 #include <vector>
 
 namespace legate::mapping::detail {
@@ -36,7 +39,7 @@ struct RegionGroup {
 
   std::set<Region> regions{};
   Domain bounding_box{};
-  std::map<const RegionGroup*, bool> subsumption_cache{};
+  std::unordered_map<const RegionGroup*, bool> subsumption_cache{};
 };
 
 std::ostream& operator<<(std::ostream& os, const RegionGroup& region_group);
@@ -88,8 +91,8 @@ class InstanceSet {  // NOLINT(bugprone-forward-declaration-namespace)
  private:
   void dump_and_sanity_check() const;
 
-  std::map<RegionGroup*, InstanceSpec> instances_{};
-  std::map<Legion::LogicalRegion, RegionGroupP> groups_{};
+  std::unordered_map<RegionGroup*, InstanceSpec> instances_{};
+  std::unordered_map<Region, RegionGroupP> groups_{};
 };
 
 class ReductionInstanceSet {
@@ -120,7 +123,7 @@ class ReductionInstanceSet {
   bool erase(const Instance& inst);
 
  private:
-  std::map<Region, ReductionInstanceSpec> instances_{};
+  std::unordered_map<Region, ReductionInstanceSpec> instances_{};
 };
 
 class BaseInstanceManager {
@@ -135,7 +138,7 @@ class BaseInstanceManager {
     FieldMemInfo(RegionTreeID t, FieldID f, Memory m);
 
     bool operator==(const FieldMemInfo& rhs) const;
-    bool operator<(const FieldMemInfo& rhs) const;
+    [[nodiscard]] size_t hash() const noexcept;
 
     RegionTreeID tid;
     FieldID fid;
@@ -174,7 +177,7 @@ class InstanceManager : public BaseInstanceManager {
   [[nodiscard]] std::map<Memory, size_t> aggregate_instance_sizes() const;
 
  private:
-  std::map<FieldMemInfo, InstanceSet> instance_sets_{};
+  std::unordered_map<FieldMemInfo, InstanceSet, hasher<FieldMemInfo>> instance_sets_{};
 };
 
 class ReductionInstanceManager : public BaseInstanceManager {
@@ -199,7 +202,7 @@ class ReductionInstanceManager : public BaseInstanceManager {
   [[nodiscard]] static ReductionInstanceManager* get_instance_manager();
 
  private:
-  std::map<FieldMemInfo, ReductionInstanceSet> instance_sets_{};
+  std::unordered_map<FieldMemInfo, ReductionInstanceSet, hasher<FieldMemInfo>> instance_sets_{};
 };
 
 }  // namespace legate::mapping::detail

src/core/mapping/detail/instance_manager.inl

@@ -61,21 +61,9 @@ inline bool BaseInstanceManager::FieldMemInfo::operator==(const FieldMemInfo& rh
   return tid == rhs.tid && fid == rhs.fid && memory == rhs.memory;
 }
 
-inline bool BaseInstanceManager::FieldMemInfo::operator<(const FieldMemInfo& rhs) const
+inline size_t BaseInstanceManager::FieldMemInfo::hash() const noexcept
 {
-  if (tid < rhs.tid) {
-    return true;
-  }
-  if (tid > rhs.tid) {
-    return false;
-  }
-  if (fid < rhs.fid) {
-    return true;
-  }
-  if (fid > rhs.fid) {
-    return false;
-  }
-  return memory < rhs.memory;
+  return hash_all(tid, fid, memory.id);
 }
 
 inline Legion::Mapping::LocalLock& BaseInstanceManager::manager_lock() { return manager_lock_; }

src/core/mapping/machine.cc

@@ -10,10 +10,11 @@
  * its affiliates is strictly prohibited.
  */
 
-#include "core/mapping/detail/machine.h"
-
 #include "core/mapping/machine.h"
 
+#include "core/mapping/detail/machine.h"
+#include "core/utilities/hash.h"
+
 #include <iostream>
 #include <sstream>
 #include <stdexcept>
@@ -73,6 +74,8 @@ bool ProcessorRange::operator<(const ProcessorRange& other) const noexcept
   return per_node_count < other.per_node_count;
 }
 
+size_t ProcessorRange::hash() const noexcept { return hash_all(low, high, per_node_count); }
+
 std::ostream& operator<<(std::ostream& stream, const ProcessorRange& range)
 {
   stream << range.to_string();

src/core/mapping/machine.h

@@ -112,6 +112,7 @@ struct ProcessorRange {
   bool operator==(const ProcessorRange& other) const noexcept;
   bool operator!=(const ProcessorRange& other) const noexcept;
   bool operator<(const ProcessorRange& other) const noexcept;
+  [[nodiscard]] size_t hash() const noexcept;
 };
 
 std::ostream& operator<<(std::ostream& stream, const ProcessorRange& range);