greg7mdp / parallel-hashmap

A family of header-only, very fast and memory-friendly hashmap and btree containers.
https://greg7mdp.github.io/parallel-hashmap/
Apache License 2.0
2.47k stars 234 forks source link

Memory not reclaimed after calling map.clear() #237

Closed marioroy closed 5 months ago

marioroy commented 5 months ago

Hi, @greg7mdp. I recall map.clear() releasing the memory immediately. This stopped working and unsure if occurred around PR #205. So now, the memory is not reclaimed at the OS level until the application exits. I miss the prior map.clear() behavior.

I reverted a little bit and now see memory reclaimed.

diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h
index d9a5b7b..1ac94c1 100644
--- a/parallel_hashmap/phmap.h
+++ b/parallel_hashmap/phmap.h
@@ -1265,7 +1265,9 @@ public:
     PHMAP_ATTRIBUTE_REINITIALIZES void clear() {
         if (empty())
             return;
-        if (capacity_) {
+        if (capacity_ > 127) {
+            destroy_slots();
+        } else if (capacity_) {
            PHMAP_IF_CONSTEXPR((!std::is_trivially_destructible<typename PolicyTraits::value_type>::value ||
                                std::is_same<typename Policy::is_flat, std::false_type>::value)) {
                 // node map or not trivially destructible... we  need to iterate and destroy values one by one

I wish map.clear() accepted a boolean map.clear(true) to do the prior behavior if (bool_destroy && capacity_ > 127).

greg7mdp commented 5 months ago

Hi @marioroy , if you want to guarantee that your map is cleared, you can swap it with a temporary map, for example:

MyMap map;
map.emplace(....);
...
MyMap().swap(map); // swap map with an empty temporary, which is immediately destroyed
marioroy commented 5 months ago

Thank you. Your suggestion works.

marioroy commented 5 months ago

Hi, @greg7mdp. I met to ask another question. Is it possible to clear/destroy a sub map individually, inside the top-level parallel loop? This will help minimize total memory consumption.

   // Store the properties into a vector
   vec_str_int_type propvec;
   size_t prev_num_keys;

   propvec.resize(map.size());
   std::vector<vec_str_int_type::iterator> I; I.resize(map.subcnt());
   I[0] = propvec.begin();

   for (size_t i = 0; i < map.subcnt(); ++i) {
      map.with_submap(i, [&](const map_str_int_type::EmbeddedSet& set) {
         if (i == 0) {
            prev_num_keys = set.size();
         } else {
            I[i] = I[i-1] + prev_num_keys;
            prev_num_keys = set.size();
         }
      });
   }  

   #pragma omp parallel for schedule(static, 1) num_threads(4)
   for (size_t i = 0; i < map.subcnt(); ++i) {
      map.with_submap(i, [&](const map_str_int_type::EmbeddedSet& set) {
         auto it = I[i];
         for (auto& x : set)
            *it++ = std::make_pair(std::move(x.first), x.second);
         // here, I like to reclaim memory after the inner-for-loop completes
      });
   }

   // swap map with an empty temporary, which is immediately destroyed
   map_str_int_type().swap(map);
marioroy commented 5 months ago

Ah, the vector is already fully allocated initially. Never mind. The maximum memory consumption is reached.

I'm updating the llil4map demonstrations and checking gcc++, clang++, including nvc++. Nvc++ is the reason for the update. I'm resolving nvc++ slow compile and slow program output.