From 0c55705d24be5297ada2bd268ea87a034033d8e7 Mon Sep 17 00:00:00 2001
From: Stephane Del Pino <stephane.delpino44@gmail.com>
Date: Sun, 27 Feb 2022 11:26:50 +0100
Subject: [PATCH] Reduce memory use and allocations for some connectivity
 descriptors

Memory use reduction occurs for 1D and 2D
- In dimension 1 node/edge/face numbers/owner/is_owned share the same
underlying arrays since node, edge and face are actually the same
- In dimension 2 edge/face numbers/owner/is_owned share the same
underlying arrays since edge and face are actually the same

On the way, ItemValue (resp. ItemArray) APIs changed: one can no more
affect a direct Array (resp. Table). One now must use the new
constructor that uses a connectivity and an Array (resp. Table). This
is always a better approach than before since it eventually consumes
one useless allocation/deallocation.
---
 src/mesh/Connectivity.cpp           | 125 ++++++----------------------
 src/mesh/ConnectivityDispatcher.cpp |   3 +-
 src/mesh/ItemArray.hpp              |  29 ++-----
 src/mesh/ItemValue.hpp              |  27 ++----
 tests/test_ItemArray.cpp            |  17 ++--
 tests/test_ItemValue.cpp            |  16 ++--
 6 files changed, 53 insertions(+), 164 deletions(-)

diff --git a/src/mesh/Connectivity.cpp b/src/mesh/Connectivity.cpp
index a542c35e0..cc501c1ba 100644
--- a/src/mesh/Connectivity.cpp
+++ b/src/mesh/Connectivity.cpp
@@ -31,17 +31,10 @@ Connectivity<Dimension>::_buildFrom(const ConnectivityDescriptor& descriptor)
     m_cell_type = cell_type;
   }
 
-  {
-    WeakCellValue<int> cell_number(*this);
-    cell_number   = convert_to_array(descriptor.cell_number_vector);
-    m_cell_number = cell_number;
-  }
+  m_cell_number = WeakCellValue<int>(*this, convert_to_array(descriptor.cell_number_vector));
 
-  {
-    WeakNodeValue<int> node_number(*this);
-    node_number   = convert_to_array(descriptor.node_number_vector);
-    m_node_number = node_number;
-  }
+  Array node_number_array = convert_to_array(descriptor.node_number_vector);
+  m_node_number           = WeakNodeValue<int>(*this, node_number_array);
 
   {
     WeakCellValue<int> cell_global_index(*this);
@@ -51,11 +44,7 @@ Connectivity<Dimension>::_buildFrom(const ConnectivityDescriptor& descriptor)
     m_cell_global_index = cell_global_index;
   }
 
-  {
-    WeakCellValue<int> cell_owner(*this);
-    cell_owner   = convert_to_array(descriptor.cell_owner_vector);
-    m_cell_owner = cell_owner;
-  }
+  m_cell_owner = WeakCellValue<int>(*this, convert_to_array(descriptor.cell_owner_vector));
 
   {
     const int rank = parallel::rank();
@@ -65,15 +54,13 @@ Connectivity<Dimension>::_buildFrom(const ConnectivityDescriptor& descriptor)
     m_cell_is_owned = cell_is_owned;
   }
 
-  {
-    WeakNodeValue<int> node_owner(*this);
-    node_owner   = convert_to_array(descriptor.node_owner_vector);
-    m_node_owner = node_owner;
-  }
+  Array node_owner_array = convert_to_array(descriptor.node_owner_vector);
+  m_node_owner           = WeakNodeValue<int>{*this, node_owner_array};
 
+  Array<bool> node_is_owned_array(this->numberOfNodes());
   {
     const int rank = parallel::rank();
-    WeakNodeValue<bool> node_is_owned(*this);
+    WeakNodeValue<bool> node_is_owned(*this, node_is_owned_array);
     parallel_for(
       this->numberOfNodes(), PUGS_LAMBDA(NodeId r) { node_is_owned[r] = (m_node_owner[r] == rank); });
     m_node_is_owned = node_is_owned;
@@ -84,46 +71,14 @@ Connectivity<Dimension>::_buildFrom(const ConnectivityDescriptor& descriptor)
 
   if constexpr (Dimension == 1) {
     // faces are similar to nodes
-    {
-      WeakFaceValue<int> face_number(*this);
-      face_number   = convert_to_array(descriptor.node_number_vector);
-      m_face_number = face_number;
-    }
-
-    {
-      WeakFaceValue<int> face_owner(*this);
-      face_owner   = convert_to_array(descriptor.node_owner_vector);
-      m_face_owner = face_owner;
-    }
-
-    {
-      const int rank = parallel::rank();
-      WeakFaceValue<bool> face_is_owned(*this);
-      parallel_for(
-        this->numberOfFaces(), PUGS_LAMBDA(FaceId l) { face_is_owned[l] = (m_face_owner[l] == rank); });
-      m_face_is_owned = face_is_owned;
-    }
+    m_face_number   = WeakFaceValue<int>(*this, node_number_array);
+    m_face_owner    = WeakFaceValue<int>(*this, node_owner_array);
+    m_face_is_owned = WeakFaceValue<bool>(*this, node_is_owned_array);
 
     // edges are similar to nodes
-    {
-      WeakEdgeValue<int> edge_number(*this);
-      edge_number   = convert_to_array(descriptor.node_number_vector);
-      m_edge_number = edge_number;
-    }
-
-    {
-      WeakEdgeValue<int> edge_owner(*this);
-      edge_owner   = convert_to_array(descriptor.node_owner_vector);
-      m_edge_owner = edge_owner;
-    }
-
-    {
-      const int rank = parallel::rank();
-      WeakEdgeValue<bool> edge_is_owned(*this);
-      parallel_for(
-        this->numberOfEdges(), PUGS_LAMBDA(EdgeId l) { edge_is_owned[l] = (m_edge_owner[l] == rank); });
-      m_edge_is_owned = edge_is_owned;
-    }
+    m_edge_number   = WeakEdgeValue<int>(*this, node_number_array);
+    m_edge_owner    = WeakEdgeValue<int>(*this, node_owner_array);
+    m_edge_is_owned = WeakEdgeValue<bool>(*this, node_is_owned_array);
   } else {
     m_item_to_item_matrix[itemTId(ItemType::face)][itemTId(ItemType::node)] = descriptor.face_to_node_vector;
 
@@ -140,21 +95,16 @@ Connectivity<Dimension>::_buildFrom(const ConnectivityDescriptor& descriptor)
       m_cell_face_is_reversed = cell_face_is_reversed;
     }
 
-    {
-      WeakFaceValue<int> face_number(*this);
-      face_number   = convert_to_array(descriptor.face_number_vector);
-      m_face_number = face_number;
-    }
+    Array face_number_array = convert_to_array(descriptor.face_number_vector);
+    m_face_number           = WeakFaceValue<int>(*this, face_number_array);
 
-    {
-      WeakFaceValue<int> face_owner(*this);
-      face_owner   = convert_to_array(descriptor.face_owner_vector);
-      m_face_owner = face_owner;
-    }
+    Array face_owner_array = convert_to_array(descriptor.face_owner_vector);
+    m_face_owner           = WeakFaceValue<int>(*this, face_owner_array);
 
+    Array<bool> face_is_owned_array(this->numberOfFaces());
     {
       const int rank = parallel::rank();
-      WeakFaceValue<bool> face_is_owned(*this);
+      WeakFaceValue<bool> face_is_owned(*this, face_is_owned_array);
       parallel_for(
         this->numberOfFaces(), PUGS_LAMBDA(FaceId l) { face_is_owned[l] = (m_face_owner[l] == rank); });
       m_face_is_owned = face_is_owned;
@@ -164,25 +114,9 @@ Connectivity<Dimension>::_buildFrom(const ConnectivityDescriptor& descriptor)
 
     if constexpr (Dimension == 2) {
       // edges are similar to faces
-      {
-        WeakEdgeValue<int> edge_number(*this);
-        edge_number   = convert_to_array(descriptor.face_number_vector);
-        m_edge_number = edge_number;
-      }
-
-      {
-        WeakEdgeValue<int> edge_owner(*this);
-        edge_owner   = convert_to_array(descriptor.face_owner_vector);
-        m_edge_owner = edge_owner;
-      }
-
-      {
-        const int rank = parallel::rank();
-        WeakEdgeValue<bool> edge_is_owned(*this);
-        parallel_for(
-          this->numberOfEdges(), PUGS_LAMBDA(EdgeId l) { edge_is_owned[l] = (m_edge_owner[l] == rank); });
-        m_edge_is_owned = edge_is_owned;
-      }
+      m_edge_number   = WeakEdgeValue<int>(*this, face_number_array);
+      m_edge_owner    = WeakEdgeValue<int>(*this, face_owner_array);
+      m_edge_is_owned = WeakEdgeValue<bool>(*this, face_is_owned_array);
 
     } else {
       m_item_to_item_matrix[itemTId(ItemType::edge)][itemTId(ItemType::node)] = descriptor.edge_to_node_vector;
@@ -202,17 +136,8 @@ Connectivity<Dimension>::_buildFrom(const ConnectivityDescriptor& descriptor)
         m_face_edge_is_reversed = face_edge_is_reversed;
       }
 
-      {
-        WeakEdgeValue<int> edge_number(*this);
-        edge_number   = convert_to_array(descriptor.edge_number_vector);
-        m_edge_number = edge_number;
-      }
-
-      {
-        WeakEdgeValue<int> edge_owner(*this);
-        edge_owner   = convert_to_array(descriptor.edge_owner_vector);
-        m_edge_owner = edge_owner;
-      }
+      m_edge_number = WeakEdgeValue<int>(*this, convert_to_array(descriptor.edge_number_vector));
+      m_edge_owner  = WeakEdgeValue<int>(*this, convert_to_array(descriptor.edge_owner_vector));
 
       {
         const int rank = parallel::rank();
diff --git a/src/mesh/ConnectivityDispatcher.cpp b/src/mesh/ConnectivityDispatcher.cpp
index 769eb23bc..8c10a5a8f 100644
--- a/src/mesh/ConnectivityDispatcher.cpp
+++ b/src/mesh/ConnectivityDispatcher.cpp
@@ -16,8 +16,7 @@ ConnectivityDispatcher<Dimension>::_buildNewOwner()
     CRSGraph connectivity_graph = m_connectivity.cellToCellGraph();
     Partitioner P;
 
-    CellValue<int> cell_new_owner(m_connectivity);
-    cell_new_owner = P.partition(connectivity_graph);
+    CellValue<int> cell_new_owner(m_connectivity, P.partition(connectivity_graph));
 
     this->_dispatchedInfo<ItemType::cell>().m_new_owner = cell_new_owner;
   } else {
diff --git a/src/mesh/ItemArray.hpp b/src/mesh/ItemArray.hpp
index 0033bb24e..9213bd313 100644
--- a/src/mesh/ItemArray.hpp
+++ b/src/mesh/ItemArray.hpp
@@ -126,28 +126,6 @@ class ItemArray
     return m_values.numberOfColumns();
   }
 
-  template <typename DataType2>
-  PUGS_INLINE ItemArray&
-  operator=(const Table<DataType2>& table) noexcept(NO_ASSERT)
-  {
-    // ensures that DataType is the same as source DataType2
-    static_assert(std::is_same_v<std::remove_const_t<DataType>, std::remove_const_t<DataType2>>,
-                  "Cannot assign ItemArray of different type");
-    // ensures that const is not lost through copy
-    static_assert(((std::is_const_v<DataType2> and std::is_const_v<DataType>) or not std::is_const_v<DataType2>),
-                  "Cannot assign ItemArray of const to ItemArray of non-const");
-
-    Assert((table.numberOfRows() * table.numberOfColumns() == 0) or this->isBuilt(),
-           "Cannot assign array of arrays to a non-built ItemArray\n");
-
-    Assert(this->numberOfItems() == table.numberOfRows(), "Cannot assign a table of a different dimensions\n");
-    Assert(this->sizeOfArrays() == table.numberOfColumns(), "Cannot assign a table of a different dimensions\n");
-
-    m_values = table;
-
-    return *this;
-  }
-
   template <typename DataType2, typename ConnectivityPtr2>
   PUGS_INLINE ItemArray&
   operator=(const ItemArray<DataType2, item_type, ConnectivityPtr2>& array_per_item) noexcept
@@ -196,6 +174,13 @@ class ItemArray
                                                  "supported");
   }
 
+  PUGS_INLINE
+  ItemArray(const IConnectivity& connectivity, const Table<DataType>& table) noexcept(NO_ASSERT)
+    : m_connectivity_ptr{connectivity.shared_ptr()}, m_values{table}
+  {
+    Assert(connectivity.numberOf<item_type>() == table.numberOfRows(), "Invalid table, wrong number of rows");
+  }
+
   PUGS_INLINE
   ~ItemArray() = default;
 };
diff --git a/src/mesh/ItemValue.hpp b/src/mesh/ItemValue.hpp
index 1b888e704..e6df4b705 100644
--- a/src/mesh/ItemValue.hpp
+++ b/src/mesh/ItemValue.hpp
@@ -117,26 +117,6 @@ class ItemValue
     static_assert(std::is_same_v<IndexType, ItemId>, "ItemValue must be indexed by ItemId");
   }
 
-  template <typename DataType2>
-  PUGS_INLINE ItemValue&
-  operator=(const Array<DataType2>& values) noexcept(NO_ASSERT)
-  {
-    // ensures that DataType is the same as source DataType2
-    static_assert(std::is_same_v<std::remove_const_t<DataType>, std::remove_const_t<DataType2>>,
-                  "Cannot assign ItemValue of different type");
-    // ensures that const is not lost through copy
-    static_assert(((std::is_const_v<DataType2> and std::is_const_v<DataType>) or not std::is_const_v<DataType2>),
-                  "Cannot assign ItemValue of const to ItemValue of non-const");
-
-    Assert((m_values.size() == values.size()), "Cannot assign an array of values of a different size\n");
-
-    Assert((values.size() == 0) or this->isBuilt(), "Cannot assign array of values to a non-built ItemValue\n");
-
-    m_values = values;
-
-    return *this;
-  }
-
   template <typename DataType2, typename ConnectivityPtr2>
   PUGS_INLINE ItemValue&
   operator=(const ItemValue<DataType2, item_type, ConnectivityPtr2>& value_per_item) noexcept
@@ -184,6 +164,13 @@ class ItemValue
                                                  "supported");
   }
 
+  PUGS_INLINE
+  ItemValue(const IConnectivity& connectivity, const Array<DataType>& values) noexcept(NO_ASSERT)
+    : m_connectivity_ptr{connectivity.shared_ptr()}, m_values{values}
+  {
+    Assert((m_values.size() == connectivity.numberOf<item_type>()), "Invalid values size");
+  }
+
   PUGS_INLINE
   ~ItemValue() = default;
 };
diff --git a/tests/test_ItemArray.cpp b/tests/test_ItemArray.cpp
index cf59acf90..b98c12ba2 100644
--- a/tests/test_ItemArray.cpp
+++ b/tests/test_ItemArray.cpp
@@ -145,9 +145,7 @@ TEST_CASE("ItemArray", "[mesh]")
 
         const Connectivity<3>& connectivity = mesh_3d->connectivity();
 
-        CellArray<size_t> cell_array{connectivity, 3};
-
-        Table<size_t> table{cell_array.numberOfItems(), cell_array.sizeOfArrays()};
+        Table<size_t> table{connectivity.numberOfCells(), 3};
         {
           size_t k = 0;
           for (size_t i = 0; i < table.numberOfRows(); ++i) {
@@ -156,7 +154,8 @@ TEST_CASE("ItemArray", "[mesh]")
             }
           }
         }
-        cell_array = table;
+
+        CellArray<size_t> cell_array{connectivity, table};
 
         auto is_same = [](const CellArray<size_t>& cell_array, const Table<size_t>& table) {
           bool is_same = true;
@@ -170,6 +169,7 @@ TEST_CASE("ItemArray", "[mesh]")
         };
 
         REQUIRE(is_same(cell_array, table));
+        REQUIRE(&(cell_array[CellId{0}][0]) == &(table(0, 0)));
       }
     }
   }
@@ -282,8 +282,7 @@ TEST_CASE("ItemArray", "[mesh]")
       }
     }
 
-    CellArray<int> cell_array{mesh->connectivity(), 3};
-    cell_array = table;
+    CellArray<int> cell_array{mesh->connectivity(), table};
 
     std::ostringstream table_ost;
     table_ost << table;
@@ -351,10 +350,8 @@ TEST_CASE("ItemArray", "[mesh]")
 
           const Connectivity<3>& connectivity = mesh_3d->connectivity();
 
-          CellArray<size_t> cell_array{connectivity, 2};
-
-          Table<size_t> values{3, connectivity.numberOfCells() + 3};
-          REQUIRE_THROWS_AS(cell_array = values, AssertError);
+          Table<size_t> values{connectivity.numberOfCells() + 3, 3};
+          REQUIRE_THROWS_WITH(CellArray<size_t>(connectivity, values), "Invalid table, wrong number of rows");
         }
       }
     }
diff --git a/tests/test_ItemValue.cpp b/tests/test_ItemValue.cpp
index fcea42470..be1307593 100644
--- a/tests/test_ItemValue.cpp
+++ b/tests/test_ItemValue.cpp
@@ -121,14 +121,12 @@ TEST_CASE("ItemValue", "[mesh]")
 
         const Connectivity<3>& connectivity = mesh_3d->connectivity();
 
-        CellValue<size_t> cell_value{connectivity};
-
-        Array<size_t> values{cell_value.numberOfItems()};
+        Array<size_t> values{connectivity.numberOfCells()};
         for (size_t i = 0; i < values.size(); ++i) {
           values[i] = i;
         }
 
-        cell_value = values;
+        CellValue<size_t> cell_value{connectivity, values};
 
         {
           bool is_same = true;
@@ -136,6 +134,7 @@ TEST_CASE("ItemValue", "[mesh]")
             is_same &= (cell_value[i_cell] == i_cell);
           }
           REQUIRE(is_same);
+          REQUIRE(&(cell_value[CellId{0}]) == &(values[0]));
         }
       }
     }
@@ -227,8 +226,7 @@ TEST_CASE("ItemValue", "[mesh]")
       array[i] = 2 * i + 1;
     }
 
-    CellValue<int> cell_value{mesh->connectivity()};
-    cell_value = array;
+    CellValue<int> cell_value{mesh->connectivity(), array};
 
     std::ostringstream array_ost;
     array_ost << array;
@@ -296,10 +294,8 @@ TEST_CASE("ItemValue", "[mesh]")
 
           const Connectivity<3>& connectivity = mesh_3d->connectivity();
 
-          CellValue<size_t> cell_value{connectivity};
-
-          Array<size_t> values{3 + cell_value.numberOfItems()};
-          REQUIRE_THROWS_AS(cell_value = values, AssertError);
+          Array<size_t> values{3 + connectivity.numberOfCells()};
+          REQUIRE_THROWS_WITH(CellValue<size_t>(connectivity, values), "Invalid values size");
         }
       }
     }
-- 
GitLab