From 1229af023cf6165e20edb4f3bf0f844b06eeb51e Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Tue, 22 Aug 2023 15:06:50 -0700
Subject: [PATCH 01/17] scheduled_point_rb_node_t: add ==, != operators

Problem: `scheduled_point_rb_node_t` needs `==` and `!=` operators to
simplify and improve equality testing for planner updates, assignment,
and copy construction.

Add the operators.
---
 resource/planner/c++/scheduled_point_tree.cpp | 12 ++++++++++++
 resource/planner/c++/scheduled_point_tree.hpp |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/resource/planner/c++/scheduled_point_tree.cpp b/resource/planner/c++/scheduled_point_tree.cpp
index 15f92259c..ffd287c1f 100644
--- a/resource/planner/c++/scheduled_point_tree.cpp
+++ b/resource/planner/c++/scheduled_point_tree.cpp
@@ -64,6 +64,18 @@ bool operator<(const int64_t lhs, const scheduled_point_rb_node_t &rhs) {
     return lhs < rhs.get_point ()->at;
 }
 
+bool scheduled_point_rb_node_t::operator== (
+         const scheduled_point_rb_node_t &other) const
+{
+    return this->get_point ()->at == other.get_point ()->at;
+}
+
+bool scheduled_point_rb_node_t::operator!= (
+         const scheduled_point_rb_node_t &other) const
+{
+    return !operator == (other);
+}
+
 
 /*******************************************************************************
  *                                                                             *
diff --git a/resource/planner/c++/scheduled_point_tree.hpp b/resource/planner/c++/scheduled_point_tree.hpp
index bd87a2f20..617f19640 100644
--- a/resource/planner/c++/scheduled_point_tree.hpp
+++ b/resource/planner/c++/scheduled_point_tree.hpp
@@ -43,6 +43,8 @@ struct scheduled_point_rb_node_t
            : public rb_node_base_t,
              public ygg::RBTreeNodeBase<scheduled_point_rb_node_t> {
     bool operator< (const scheduled_point_rb_node_t &other) const;
+    bool operator== (const scheduled_point_rb_node_t &other) const;
+    bool operator!= (const scheduled_point_rb_node_t &other) const;
 };
 
 using scheduled_point_rb_tree_t = ygg::RBTree<scheduled_point_rb_node_t,

From ba74958074cb5977dd9c0c6ece61e4da4464b491 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Tue, 22 Aug 2023 15:10:00 -0700
Subject: [PATCH 02/17] mt_resource_rb_node_t: add ==, != operators

Problem: `mt_resource_rb_node_t` needs `==` and `!=` operators to
simplify and improve equality testing for planner updates, assignment,
and copy construction.

Add the operators.
---
 resource/planner/c++/mintime_resource_tree.cpp | 11 +++++++++++
 resource/planner/c++/mintime_resource_tree.hpp |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/resource/planner/c++/mintime_resource_tree.cpp b/resource/planner/c++/mintime_resource_tree.cpp
index d38b3f72c..252f0824b 100644
--- a/resource/planner/c++/mintime_resource_tree.cpp
+++ b/resource/planner/c++/mintime_resource_tree.cpp
@@ -207,6 +207,17 @@ bool mt_resource_rb_node_t::operator< (const mt_resource_rb_node_t &other) const
     return this->remaining < other.remaining;
 }
 
+bool mt_resource_rb_node_t::operator== (const mt_resource_rb_node_t &other) const
+{
+    return this->remaining == other.remaining;
+}
+
+bool mt_resource_rb_node_t::operator!= (const mt_resource_rb_node_t &other) const
+{
+    return !operator == (other);
+}
+
+
 
 /*******************************************************************************
  *                                                                             *
diff --git a/resource/planner/c++/mintime_resource_tree.hpp b/resource/planner/c++/mintime_resource_tree.hpp
index cd74dd36d..e98521250 100644
--- a/resource/planner/c++/mintime_resource_tree.hpp
+++ b/resource/planner/c++/mintime_resource_tree.hpp
@@ -24,6 +24,8 @@ struct mt_resource_rb_node_t
     int64_t subtree_min;
     int64_t remaining;
     bool operator< (const mt_resource_rb_node_t &other) const;
+    bool operator== (const mt_resource_rb_node_t &other) const;
+    bool operator!= (const mt_resource_rb_node_t &other) const;
 };
 
 template <class mt_resource_rb_node_t, class NodeTraits>

From e194dd5205f9b6bc8056d1f222799c745787af70 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Tue, 22 Aug 2023 14:36:23 -0700
Subject: [PATCH 03/17] planner: add == and != operators for scheduled_point_t

Problem: `scheduled_point_t` needs `==` and `!=` operators to simplify
and improve equality testing for planner updates, assignment, and copy
construction.

Add the operators and the requisite .cpp file.
---
 resource/planner/c++/CMakeLists.txt           |  1 +
 resource/planner/c++/Makefile.am              |  3 +-
 .../planner/c++/planner_internal_tree.cpp     | 49 +++++++++++++++++++
 .../planner/c++/planner_internal_tree.hpp     |  3 ++
 4 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 resource/planner/c++/planner_internal_tree.cpp

diff --git a/resource/planner/c++/CMakeLists.txt b/resource/planner/c++/CMakeLists.txt
index 1858ae272..b080f7b5d 100644
--- a/resource/planner/c++/CMakeLists.txt
+++ b/resource/planner/c++/CMakeLists.txt
@@ -3,6 +3,7 @@ add_library(planner_cxx STATIC
 	./scheduled_point_tree.cpp
 	./mintime_resource_tree.cpp
 	./planner_multi.cpp
+	./planner_internal_tree.cpp
 	./mintime_resource_tree.hpp
 	./planner_internal_tree.hpp
 	./scheduled_point_tree.hpp
diff --git a/resource/planner/c++/Makefile.am b/resource/planner/c++/Makefile.am
index fbd0ceba9..0fc7ef23f 100644
--- a/resource/planner/c++/Makefile.am
+++ b/resource/planner/c++/Makefile.am
@@ -33,7 +33,8 @@ libplanner_cxx_la_SOURCES = \
         planner.cpp \
         planner_multi.cpp \
         scheduled_point_tree.cpp \
-        mintime_resource_tree.cpp
+        mintime_resource_tree.cpp \
+        planner_internal_tree.cpp
 
 libplanner_cxx_la_CFLAGS = $(AM_CFLAGS) -I$(top_srcdir)/resource/planner/c++
 
diff --git a/resource/planner/c++/planner_internal_tree.cpp b/resource/planner/c++/planner_internal_tree.cpp
new file mode 100644
index 000000000..7cbffe2bb
--- /dev/null
+++ b/resource/planner/c++/planner_internal_tree.cpp
@@ -0,0 +1,49 @@
+/*****************************************************************************\
+ * Copyright 2024 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, LICENSE)
+ *
+ * This file is part of the Flux resource manager framework.
+ * For details, see https://github.com/flux-framework.
+ *
+ * SPDX-License-Identifier: LGPL-3.0
+\*****************************************************************************/
+
+extern "C" {
+#if HAVE_CONFIG_H
+#include "config.h"
+#endif
+}
+
+#include "planner_internal_tree.hpp"
+
+bool scheduled_point_t::operator== (const scheduled_point_t &o) const
+{
+    if (point_rb != o.point_rb)
+        return false;
+    if (resource_rb != o.resource_rb)
+        return false;
+    if (at != o.at)
+        return false;
+    if (in_mt_resource_tree != o.in_mt_resource_tree)
+        return false;
+    if (new_point != o.new_point)
+        return false;
+    if (ref_count != o.ref_count)
+        return false;
+    if (remaining != o.remaining)
+        return false;
+    if (scheduled != o.scheduled)
+        return false;
+
+    return true;
+}
+
+bool scheduled_point_t::operator!= (const scheduled_point_t &o) const
+{
+    return !operator == (o);
+}
+
+
+/*
+ * vi: ts=4 sw=4 expandtab
+ */
diff --git a/resource/planner/c++/planner_internal_tree.hpp b/resource/planner/c++/planner_internal_tree.hpp
index 88a4052bc..4df95fffa 100644
--- a/resource/planner/c++/planner_internal_tree.hpp
+++ b/resource/planner/c++/planner_internal_tree.hpp
@@ -19,6 +19,9 @@
  *  tree.
  */
 struct scheduled_point_t {
+    bool operator== (const scheduled_point_t &o) const;
+    bool operator!= (const scheduled_point_t &o) const;
+
     scheduled_point_rb_node_t point_rb; /* BST node for scheduled point tree */
     mt_resource_rb_node_t resource_rb;  /* BST node for min-time resource tree */
     int64_t at;                  /* Resource-state changing time */

From 8b93e514e94913e369ce9c9f1fb1a920c054ba56 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Tue, 22 Aug 2023 15:05:29 -0700
Subject: [PATCH 04/17] planner: replace scheduled_points_equal with ==

Problem: `scheduled_point_t` equality testing should be done via
operator overloading, not by a `planner` member function.

Remove `scheduled_points_equal` and perform `scheduled_point_t` equality
testing through the class comparison operators.
---
 resource/planner/c++/planner.cpp | 32 +++++---------------------------
 resource/planner/c++/planner.hpp |  2 --
 2 files changed, 5 insertions(+), 29 deletions(-)

diff --git a/resource/planner/c++/planner.cpp b/resource/planner/c++/planner.cpp
index 87e2166d1..326cea32d 100644
--- a/resource/planner/c++/planner.cpp
+++ b/resource/planner/c++/planner.cpp
@@ -118,7 +118,7 @@ bool planner::operator== (const planner &o) const
         return false;
     // m_p0 or o.m_p0 could be uninitialized
     if (m_p0 && o.m_p0) {
-        if (!scheduled_points_equal (*m_p0, *(o.m_p0)))
+        if (*m_p0 != *(o.m_p0))
             return false;
     } else if (m_p0 || o.m_p0) {
         return false;
@@ -428,25 +428,6 @@ int planner::copy_maps (const planner &o)
     return rc;
 }
 
-bool planner::scheduled_points_equal (const scheduled_point_t &lhs,
-                                      const scheduled_point_t &rhs) const
-{
-    if (lhs.at != rhs.at)
-        return false;
-    if (lhs.in_mt_resource_tree != rhs.in_mt_resource_tree)
-        return false;
-    if (lhs.new_point != rhs.new_point)
-        return false;
-    if (lhs.ref_count != rhs.ref_count)
-        return false;
-    if (lhs.remaining != rhs.remaining)
-        return false;
-    if (lhs.scheduled != rhs.scheduled)
-        return false;
-
-    return true;
-}
-
 bool planner::span_lookups_equal (const planner &o) const
 {
     if (m_span_lookup.size () != o.m_span_lookup.size ())
@@ -472,11 +453,9 @@ bool planner::span_lookups_equal (const planner &o) const
                 return false;
             if (this_it.second->in_system != other->second->in_system)
                 return false;
-            if (!scheduled_points_equal (*(this_it.second->start_p),
-                                         *(other->second->start_p)))
+            if (*(this_it.second->start_p) != *(other->second->start_p))
                 return false;
-            if (!scheduled_points_equal (*(this_it.second->last_p),
-                                         *(other->second->last_p)))
+            if (*(this_it.second->last_p) != *(other->second->last_p))
                 return false;
         }
     }
@@ -497,8 +476,7 @@ bool planner::avail_time_iters_equal (const planner &o) const
             if (this_it.first != other->first)
                 return false;
             if (this_it.second && other->second) {
-                if (!scheduled_points_equal (*(this_it.second),
-                                             *(other->second)))
+                if (*(this_it.second) != *(other->second))
                     return false;
             } else if (this_it.second || other->second) {
                 return false;
@@ -518,7 +496,7 @@ bool planner::trees_equal (const planner &o) const
         scheduled_point_t *o_pt =
                         o.m_sched_point_tree.get_state (o.m_plan_start);
         while (this_pt) {
-            if (!scheduled_points_equal (*this_pt, *o_pt))
+            if (*this_pt != *o_pt)
                 return false;
             this_pt = m_sched_point_tree.next (this_pt);
             o_pt = o.m_sched_point_tree.next (o_pt);
diff --git a/resource/planner/c++/planner.hpp b/resource/planner/c++/planner.hpp
index 716091e0b..fd0084c6e 100644
--- a/resource/planner/c++/planner.hpp
+++ b/resource/planner/c++/planner.hpp
@@ -108,8 +108,6 @@ class planner {
     // Private class utilities
     int copy_trees (const planner &o);
     int copy_maps (const planner &o);
-    bool scheduled_points_equal (const scheduled_point_t &lhs,
-                                 const scheduled_point_t &rhs) const;
     bool span_lookups_equal (const planner &o) const;
     bool avail_time_iters_equal (const planner &o) const;
     bool trees_equal (const planner &o) const;

From ac39401a49e6d5045a6f006e027a8af9fd0d4fb0 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Tue, 22 Aug 2023 15:09:18 -0700
Subject: [PATCH 05/17] planner: add span_t == op

Problem: `span_t` equality testing should be done via
operator overloading, not in a `planner` member function.

Add a `span_t` struct equality comparison operator.
---
 resource/planner/c++/planner.cpp | 48 ++++++++++++++++++++++----------
 resource/planner/c++/planner.hpp |  3 ++
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/resource/planner/c++/planner.cpp b/resource/planner/c++/planner.cpp
index 326cea32d..a15c93949 100644
--- a/resource/planner/c++/planner.cpp
+++ b/resource/planner/c++/planner.cpp
@@ -21,6 +21,37 @@ extern "C" {
 #include "planner.hpp"
 
 
+/****************************************************************************
+ *                                                                          *
+ *                     Public Span_t Methods                                *
+ *                                                                          *
+ ****************************************************************************/
+
+bool span_t::operator== (const span_t &o) const
+{
+    if (start != o.start)
+        return false;
+    if (last != o.last)
+        return false;
+    if (span_id != o.span_id)
+        return false;
+    if (planned != o.planned)
+        return false;
+    if (in_system != o.in_system)
+        return false;
+    if ((*(start_p) != *(o.start_p)))
+        return false;
+    if ((*(last_p) != *(o.last_p)))
+        return false;
+
+    return true;
+}
+
+bool span_t::operator!= (const span_t &o) const
+{
+    return !operator == (o);
+}
+
 /****************************************************************************
  *                                                                          *
  *                     Public Planner Methods                               *
@@ -441,21 +472,8 @@ bool planner::span_lookups_equal (const planner &o) const
                 return false;
             if (this_it.first != other->first)
                 return false;
-            if (this_it.second->start != other->second->start)
-                return false;
-            if (this_it.second->last != other->second->last)
-                return false;
-            if (this_it.second->span_id != other->second->span_id)
-                return false;
-            if (this_it.second->planned != other->second->planned)
-                return false;
-            if (this_it.second->in_system != other->second->in_system)
-                return false;
-            if (this_it.second->in_system != other->second->in_system)
-                return false;
-            if (*(this_it.second->start_p) != *(other->second->start_p))
-                return false;
-            if (*(this_it.second->last_p) != *(other->second->last_p))
+            // Compare span_t
+            if (*(this_it.second) != *(other->second))
                 return false;
         }
     }
diff --git a/resource/planner/c++/planner.hpp b/resource/planner/c++/planner.hpp
index fd0084c6e..abdd2a3a8 100644
--- a/resource/planner/c++/planner.hpp
+++ b/resource/planner/c++/planner.hpp
@@ -23,6 +23,9 @@ struct request_t {
 /*! Node in a span interval tree to enable fast retrieval of intercepting spans.
  */
 struct span_t {
+    bool operator== (const span_t &o) const;
+    bool operator!= (const span_t &o) const;
+
     int64_t start;               /* start time of the span */
     int64_t last;                /* end time of the span */
     int64_t span_id;             /* unique span id */

From 7edf250456c025a912515f7fb359f19b9f3f2f82 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Wed, 20 Dec 2023 09:25:31 -0800
Subject: [PATCH 06/17] planner: add != operator

Problem: `planner` equality testing needs a convenience != operator.

Add a `planner` `!=` operator.
---
 resource/planner/c++/planner.cpp | 5 +++++
 resource/planner/c++/planner.hpp | 1 +
 2 files changed, 6 insertions(+)

diff --git a/resource/planner/c++/planner.cpp b/resource/planner/c++/planner.cpp
index a15c93949..0f55c0dc5 100644
--- a/resource/planner/c++/planner.cpp
+++ b/resource/planner/c++/planner.cpp
@@ -163,6 +163,11 @@ bool planner::operator== (const planner &o) const
     return true;
 }
 
+bool planner::operator!= (const planner &o) const
+{
+    return !operator == (o);
+}
+
 planner::~planner ()
 {
     // Destructor is nothrow
diff --git a/resource/planner/c++/planner.hpp b/resource/planner/c++/planner.hpp
index abdd2a3a8..05fc39f75 100644
--- a/resource/planner/c++/planner.hpp
+++ b/resource/planner/c++/planner.hpp
@@ -45,6 +45,7 @@ class planner {
     planner (const planner &o);
     planner &operator= (const planner &o);
     bool operator== (const planner &o) const;
+    bool operator!= (const planner &o) const;
     ~planner ();
     // Public class utilities
     int erase ();

From ab9e97aed3a32f70b2d7a6fef36517ab9bd919b0 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Wed, 10 Jan 2024 10:39:56 -0800
Subject: [PATCH 07/17] planner_multi: add != operator

Problem: `planner_multi` equality testing needs a convenience !=
operator.

Add a `planner_multi` `!=` operator.
---
 resource/planner/c++/planner_multi.cpp | 5 +++++
 resource/planner/c++/planner_multi.hpp | 1 +
 2 files changed, 6 insertions(+)

diff --git a/resource/planner/c++/planner_multi.cpp b/resource/planner/c++/planner_multi.cpp
index ff7c1966b..58e18f820 100644
--- a/resource/planner/c++/planner_multi.cpp
+++ b/resource/planner/c++/planner_multi.cpp
@@ -178,6 +178,11 @@ bool planner_multi::operator== (const planner_multi &o) const
     return true;
 }
 
+bool planner_multi::operator!= (const planner_multi &o) const
+{
+    return !operator == (o);
+}
+
 void planner_multi::erase ()
 {
     if (!m_planners.empty ()) {
diff --git a/resource/planner/c++/planner_multi.hpp b/resource/planner/c++/planner_multi.hpp
index d27122563..8e873b87e 100644
--- a/resource/planner/c++/planner_multi.hpp
+++ b/resource/planner/c++/planner_multi.hpp
@@ -28,6 +28,7 @@ class planner_multi {
     planner_multi (const planner_multi &o);
     planner_multi &operator= (const planner_multi &o);
     bool operator== (const planner_multi &o) const;
+    bool operator!= (const planner_multi &o) const;
     void erase ();
     ~planner_multi ();
 

From 84a5ad4f4aa49f34b863235e0e197245bcd53830 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Wed, 24 Jan 2024 18:02:04 -0800
Subject: [PATCH 08/17] planner_multi: use boost multi_index, update c
 interface

Problem: to update the Fluxion resource graph, the traverser will need
to be reinitialized, which assembles vectors of resource counts and
types. The ordering of the counts and types depends on how graph is
mutated. Currently, counts and types are stored as separate vectors in
`planner_multi` under the assumption that their order and size don't
change. Removing that assumption will cause inconsistency in
scheduling.

Add a `boost::multi_index` container to replace and simplify
`planner_multi` member data. The `boost::multi_index` container has the
important capability of searching and comparing by multiple indices
which can be vector-like (`random_access`), and hash-like
(`hashed_unique`). Adding two indexes for lookups enables support for
altering resource types, counts, and their orders (indices) in a
natural way. Performing these transformations with `vectors` would be
high-complexity both in time and code.
---
 resource/planner/c++/planner_multi.cpp        | 169 +++++++-----------
 resource/planner/c++/planner_multi.hpp        |  51 ++++--
 resource/planner/c/planner_multi.h            |   2 +-
 .../planner/c/planner_multi_c_interface.cpp   |  66 +++----
 resource/traversers/dfu_impl.hpp              |   7 +-
 5 files changed, 133 insertions(+), 162 deletions(-)

diff --git a/resource/planner/c++/planner_multi.cpp b/resource/planner/c++/planner_multi.cpp
index 58e18f820..9753a2658 100644
--- a/resource/planner/c++/planner_multi.cpp
+++ b/resource/planner/c++/planner_multi.cpp
@@ -35,66 +35,53 @@ planner_multi::planner_multi (int64_t base_time, uint64_t duration,
                               const char **resource_types, size_t len)
 {
     size_t i = 0;
-    char *type = nullptr;
+    std::string type;
     planner_t *p = nullptr;
 
     m_iter.on_or_after = 0;
     m_iter.duration = 0;
     for (i = 0; i < len; ++i) {
-        m_resource_totals.push_back (resource_totals[i]);
-        if ( (type = strdup (resource_types[i])) == nullptr) {
-            errno = ENOMEM;
-            throw std::runtime_error ("ERROR in strdup\n");
-        }
-        m_resource_types.push_back (type);
-        m_iter.counts.push_back (0);
         try {
-           p = new planner_t (base_time, duration,
-                              resource_totals[i],
-                              resource_types[i]);
+            type = std::string (resource_types[i]);
+            p = new planner_t (base_time, duration,
+                               resource_totals[i],
+                               resource_types[i]);
         } catch (std::bad_alloc &e) {
            errno = ENOMEM;
+           throw std::bad_alloc ();
         }
-        if (p == nullptr)
-           throw std::runtime_error ("ERROR in planner_multi ctor\n");
-        m_planners.push_back (p);
+        m_iter.counts.push_back (0);
+        m_types_totals_planners.push_back ({type, resource_totals[i], p});
     }
     m_span_counter = 0;
-
 }
 
 planner_multi::planner_multi (const planner_multi &o)
 {
-    for (size_t i = 0; i < o.m_planners.size (); ++i) {
+    for (auto iter : o.m_types_totals_planners) {
         planner_t *np = nullptr;
-        if (o.m_planners[i]) {
+        if (iter.planner) {
             try {
-                np = new planner_t (*(o.m_planners[i]->plan));
+                np = new planner_t (*(iter.planner->plan));
             } catch (std::bad_alloc &e) {
                 errno = ENOMEM;
             }
+            // planner copy ctor can throw runtime_error, resulting in nullptr
             if (np == nullptr)
-                throw std::runtime_error ("ERROR in planner_copy\n");
+                throw std::runtime_error ("ERROR in planner copy ctor"
+                                          " in planner_multi copy"
+                                          " constructor\n");
         } else {
             try {
                 np = new planner_t ();
             } catch (std::bad_alloc &e) {
                 errno = ENOMEM;
+                throw std::bad_alloc ();
             }
-            if (np == nullptr)
-                throw std::runtime_error ("ERROR in planner_new_empty\n");
-        }
-        m_planners.push_back (np);
-    }
-    char *type = nullptr;
-    for (size_t i = 0; i < o.m_resource_types.size (); ++i) {
-        if ( (type = strdup (o.m_resource_types[i])) == nullptr) {
-            errno = ENOMEM;
-            throw std::runtime_error ("ERROR in strdup; ctor\n");
         }
-        m_resource_types.push_back (type);
+        m_types_totals_planners.push_back ({iter.resource_type,
+                                            iter.resource_total, np});
     }
-    m_resource_totals = o.m_resource_totals;
     m_span_lookup = o.m_span_lookup;
     m_iter = o.m_iter;
     m_span_lookup_iter = o.m_span_lookup_iter;
@@ -106,38 +93,32 @@ planner_multi &planner_multi::operator= (const planner_multi &o)
     // Erase *this so the vectors are empty
     erase ();
 
-    for (size_t i = 0; i < o.m_planners.size (); ++i) {
+    for (auto iter : o.m_types_totals_planners) {
         planner_t *np = nullptr;
-        if (o.m_planners[i]) {
+        if (iter.planner) {
             try {
                 // Invoke copy constructor to avoid the assignment
-                // overload erase () penalty.
-                np = new planner_t (*(o.m_planners[i]->plan));
+                // operator erase () penalty.
+                np = new planner_t (*(iter.planner->plan));
             } catch (std::bad_alloc &e) {
                 errno = ENOMEM;
             }
+            // planner copy ctor can throw runtime_error, resulting in nullptr
             if (np == nullptr)
-                throw std::runtime_error ("ERROR in planner_copy\n");
+                throw std::runtime_error ("ERROR in planner copy ctor"
+                                          " in planner_multi assn"
+                                          " operator\n");
         } else {
             try {
                 np = new planner_t ();
             } catch (std::bad_alloc &e) {
                 errno = ENOMEM;
+                throw std::bad_alloc ();
             }
-            if (np == nullptr)
-                throw std::runtime_error ("ERROR in planner_new_empty\n");
-        }
-        m_planners.push_back (np);
-    }
-    char *type = nullptr;
-    for (size_t i = 0; i < o.m_resource_types.size (); ++i) {
-        if ( (type = strdup (o.m_resource_types[i])) == nullptr) {
-            errno = ENOMEM;
-            throw std::runtime_error ("ERROR in strdup; assn overload\n");
         }
-        m_resource_types.push_back (type);
+        m_types_totals_planners.push_back ({iter.resource_type,
+                                            iter.resource_total, np});
     }
-    m_resource_totals = o.m_resource_totals;
     m_span_lookup = o.m_span_lookup;
     m_iter = o.m_iter;
     m_span_lookup_iter = o.m_span_lookup_iter;
@@ -150,8 +131,6 @@ bool planner_multi::operator== (const planner_multi &o) const
 {
     if (m_span_counter != o.m_span_counter)
         return false;
-    if (m_resource_totals != o.m_resource_totals)
-        return false;
     if (m_span_lookup != o.m_span_lookup)
         return false;
     if (m_iter.on_or_after != o.m_iter.on_or_after)
@@ -161,17 +140,19 @@ bool planner_multi::operator== (const planner_multi &o) const
     if (m_iter.counts != o.m_iter.counts)
         return false;
 
-    if (m_resource_types.size () != o.m_resource_types.size ())
+    if (m_types_totals_planners.size () != o.m_types_totals_planners.size ())
         return false;
-    for (size_t i = 0; i < m_resource_types.size (); ++i) {
-        if (strcmp (m_resource_types[i], o.m_resource_types[i]) != 0)
+    auto &o_by_type = o.m_types_totals_planners.get<res_type> ();
+    auto &by_type = m_types_totals_planners.get<res_type> ();
+    for (auto data : by_type) {
+        auto o_data = o_by_type.find (data.resource_type);
+        if (o_data == o_by_type.end ())
             return false;
-    }
-
-    if (m_planners.size () != o.m_planners.size ())
-        return false;
-    for (size_t i = 0; i < m_planners.size (); ++i) {
-        if (!(*(m_planners[i]->plan) == *(o.m_planners[i]->plan)))
+        if (data.resource_type != o_data->resource_type)
+            return false;
+        if (data.resource_total != o_data->resource_total)
+            return false;
+        if (*(data.planner->plan) != *(o_data->planner->plan))
             return false;
     }
 
@@ -185,28 +166,15 @@ bool planner_multi::operator!= (const planner_multi &o) const
 
 void planner_multi::erase ()
 {
-    if (!m_planners.empty ()) {
-        size_t i = 0;
-        for (i = 0; i < m_planners.size (); ++i) {
-            if (m_planners[i]) {
-                delete m_planners[i];
-                m_planners[i] = nullptr;
+    if (!m_types_totals_planners.empty ()) {
+        for (auto iter : m_types_totals_planners) {
+            if (iter.planner) {
+                delete iter.planner;
+                iter.planner = nullptr;
             }
         }
     }
-    if (!m_resource_types.empty ()) {
-        size_t i = 0;
-        for (i = 0; i < m_resource_types.size (); ++i) {
-            if (m_resource_types[i]) {
-                free ((void *)m_resource_types[i]);
-                m_resource_types[i] = nullptr;
-            }
-        }
-    }
-    m_planners.clear ();
-    m_resource_types.clear ();
-    m_resource_totals.clear ();
-    m_span_lookup.clear ();
+    m_types_totals_planners.clear ();
 }
 
 planner_multi::~planner_multi ()
@@ -214,49 +182,40 @@ planner_multi::~planner_multi ()
     erase ();
 }
 
-planner_t *planner_multi::get_planners_at (size_t i)
-{
-    return m_planners.at (i);
-}
-
-std::vector<planner_t *> &planner_multi::get_planners ()
-{
-    return m_planners;
-}
-
-size_t planner_multi::get_planners_size ()
-{
-    return m_planners.size ();
-}
-
-void planner_multi::resource_totals_push_back (const uint64_t resource_total)
+planner_t *planner_multi::get_planner_at (size_t i) const
 {
-    m_resource_totals.push_back (resource_total);
+    return m_types_totals_planners.at (i).planner;
 }
 
-uint64_t planner_multi::get_resource_totals_at (size_t i)
+planner_t *planner_multi::get_planner_at (const char *type) const
 {
-    return m_resource_totals.at (i);
+    auto &by_res = m_types_totals_planners.get<res_type> ();
+    return by_res.find (std::string (type))->planner;
 }
 
-void planner_multi::resource_types_push_back (const char * resource_type)
+size_t planner_multi::get_planners_size () const
 {
-    m_resource_types.push_back (resource_type);
+    return m_types_totals_planners.size ();
 }
 
-const std::vector<const char *> planner_multi::get_resource_types ()
+int64_t planner_multi::get_resource_total_at (size_t i) const
 {
-    return m_resource_types;
+    return m_types_totals_planners.at (i).resource_total;
 }
 
-const char *planner_multi::get_resource_types_at (size_t i)
+int64_t planner_multi::get_resource_total_at (const char *type) const
 {
-    return m_resource_types.at (i);
+    auto &by_res = m_types_totals_planners.get<res_type> ();
+    auto result = by_res.find (std::string (type));
+    if (result == by_res.end ())
+        return -1;
+    else
+        return result->resource_total;
 }
 
-size_t planner_multi::get_resource_types_size ()
+const char *planner_multi::get_resource_type_at (size_t i) const
 {
-    return m_resource_types.size ();
+    return m_types_totals_planners.at (i).resource_type.c_str ();
 }
 
 struct request_multi &planner_multi::get_iter ()
diff --git a/resource/planner/c++/planner_multi.hpp b/resource/planner/c++/planner_multi.hpp
index 8e873b87e..4883e2256 100644
--- a/resource/planner/c++/planner_multi.hpp
+++ b/resource/planner/c++/planner_multi.hpp
@@ -12,6 +12,10 @@
 #define PLANNER_MULTI_HPP
 
 #include "planner.hpp"
+#include <boost/multi_index_container.hpp>
+#include <boost/multi_index/member.hpp>
+#include <boost/multi_index/random_access_index.hpp>
+#include <boost/multi_index/hashed_index.hpp>
 
 struct request_multi {
     int64_t on_or_after = 0;
@@ -19,6 +23,31 @@ struct request_multi {
     std::vector<int64_t> counts;
 };
 
+struct planner_multi_meta {
+    std::string resource_type;
+    mutable uint64_t resource_total; // Not an index; can mutate
+    planner_t *planner;
+};
+
+/* tags for accessing the corresponding indices of planner_multi_meta */
+struct idx {};
+struct res_type {};
+
+using boost::multi_index_container;
+using namespace boost::multi_index;
+typedef multi_index_container<
+    planner_multi_meta, // container data
+    indexed_by< // list of indexes
+        random_access< // analogous to vector
+            tag<idx> // index nametag
+        >,
+        hashed_unique<  // unordered_set-like; faster than ordered_unique in testing
+            tag<res_type>, // index nametag
+            member<planner_multi_meta, std::string, &planner_multi_meta::resource_type> // index's key
+        >
+    >
+> multi_container;
+
 class planner_multi {
 public:
     planner_multi ();
@@ -33,15 +62,12 @@ class planner_multi {
     ~planner_multi ();
 
     // Public getters and setters
-    planner_t *get_planners_at (size_t i);
-    std::vector<planner_t *> &get_planners ();
-    size_t get_planners_size ();
-    void resource_totals_push_back (const uint64_t resource_total);
-    uint64_t get_resource_totals_at (size_t i);
-    void resource_types_push_back (const char * resource_type);
-    const std::vector<const char *> get_resource_types ();
-    const char *get_resource_types_at (size_t i);
-    size_t get_resource_types_size ();
+    planner_t *get_planner_at (size_t i) const;
+    planner_t *get_planner_at (const char *type) const;
+    size_t get_planners_size () const;
+    int64_t get_resource_total_at (size_t i) const;
+    int64_t get_resource_total_at (const char *type) const;
+    const char *get_resource_type_at (size_t i) const;
     struct request_multi &get_iter ();
     // Span lookup functions
     std::map<uint64_t, std::vector<int64_t>> &get_span_lookup ();
@@ -55,13 +81,8 @@ class planner_multi {
     void set_span_counter (uint64_t sc);
     void incr_span_counter ();
 
-    // These need to be public to pass references, otherwise
-    // need to pass tmp variables by reference.
-    std::vector<const char *> m_resource_types;
-    std::vector<uint64_t> m_resource_totals;
-    std::vector<planner_t *> m_planners;
-
 private:
+    multi_container m_types_totals_planners;
     struct request_multi m_iter;
     std::map<uint64_t, std::vector<int64_t>> m_span_lookup;
     std::map<uint64_t, std::vector<int64_t>>::iterator m_span_lookup_iter;
diff --git a/resource/planner/c/planner_multi.h b/resource/planner/c/planner_multi.h
index 94a541df2..2dad4f4ba 100644
--- a/resource/planner/c/planner_multi.h
+++ b/resource/planner/c/planner_multi.h
@@ -83,7 +83,7 @@ void planner_multi_assign (planner_multi_t *lhs, planner_multi_t *rhs);
 int64_t planner_multi_base_time (planner_multi_t *ctx);
 int64_t planner_multi_duration (planner_multi_t *ctx);
 size_t planner_multi_resources_len (planner_multi_t *ctx);
-const char **planner_multi_resource_types (planner_multi_t *ctx);
+const char *planner_multi_resource_type_at (planner_multi_t *ctx, unsigned int i);
 const uint64_t *planner_multi_resource_totals (planner_multi_t *ctx);
 int64_t planner_multi_resource_total_at (planner_multi_t *ctx, unsigned int i);
 int64_t planner_multi_resource_total_by_type (planner_multi_t *ctx,
diff --git a/resource/planner/c/planner_multi_c_interface.cpp b/resource/planner/c/planner_multi_c_interface.cpp
index 95886ce23..3c6a07482 100644
--- a/resource/planner/c/planner_multi_c_interface.cpp
+++ b/resource/planner/c/planner_multi_c_interface.cpp
@@ -118,7 +118,7 @@ extern "C" int64_t planner_multi_base_time (planner_multi_t *ctx)
         errno = EINVAL;
         return -1;
     }
-    return planner_base_time (ctx->plan_multi->get_planners_at (0));
+    return planner_base_time (ctx->plan_multi->get_planner_at (static_cast<size_t> (0)));
 }
 
 extern "C" int64_t planner_multi_duration (planner_multi_t *ctx)
@@ -127,7 +127,7 @@ extern "C" int64_t planner_multi_duration (planner_multi_t *ctx)
         errno = EINVAL;
         return -1;
     }
-    return planner_duration (ctx->plan_multi->get_planners_at (0));
+    return planner_duration (ctx->plan_multi->get_planner_at (static_cast<size_t> (0)));
 }
 
 extern "C" size_t planner_multi_resources_len (planner_multi_t *ctx)
@@ -139,22 +139,14 @@ extern "C" size_t planner_multi_resources_len (planner_multi_t *ctx)
     return ctx->plan_multi->get_planners_size ();
 }
 
-extern "C" const char **planner_multi_resource_types (planner_multi_t *ctx)
+extern "C" const char *planner_multi_resource_type_at (planner_multi_t *ctx,
+                                                       unsigned int i)
 {
     if (!ctx) {
         errno = EINVAL;
         return nullptr;
     }
-    return &(ctx->plan_multi->m_resource_types[0]);
-}
-
-extern "C" const uint64_t *planner_multi_resource_totals (planner_multi_t *ctx)
-{
-    if (!ctx) {
-        errno = EINVAL;
-        return nullptr;
-    }
-    return &(ctx->plan_multi->m_resource_totals[0]);
+    return ctx->plan_multi->get_resource_type_at (i);
 }
 
 extern "C" int64_t planner_multi_resource_total_at (planner_multi_t *ctx,
@@ -166,7 +158,7 @@ extern "C" int64_t planner_multi_resource_total_at (planner_multi_t *ctx,
             errno = EINVAL;
             goto done;
         }
-        rc = planner_resource_total (ctx->plan_multi->get_planners_at (i));
+        rc = ctx->plan_multi->get_resource_total_at (i);
     }
 done:
     return rc;
@@ -175,17 +167,12 @@ extern "C" int64_t planner_multi_resource_total_at (planner_multi_t *ctx,
 extern "C" int64_t planner_multi_resource_total_by_type (
                        planner_multi_t *ctx, const char *resource_type)
 {
-    size_t i = 0;
     int64_t rc = -1;
     if (!ctx || !resource_type)
         goto done;
-    for (i = 0; i < ctx->plan_multi->get_planners_size (); i++) {
-        if ( !(strcmp (ctx->plan_multi->get_resource_types_at (i), resource_type))) {
-            rc = planner_resource_total (ctx->plan_multi->get_planners_at (i));
-            break;
-        }
-    }
-    if (i == ctx->plan_multi->get_planners_size ())
+
+    rc = ctx->plan_multi->get_resource_total_at (resource_type);
+    if (rc == -1)
         errno = EINVAL;
 done:
     return rc;
@@ -202,7 +189,7 @@ extern "C" int planner_multi_reset (planner_multi_t *ctx,
     }
 
     for (i = 0; i < ctx->plan_multi->get_planners_size (); ++i)
-        if (planner_reset (ctx->plan_multi->get_planners_at (i), base_time, duration) == -1)
+        if (planner_reset (ctx->plan_multi->get_planner_at (i), base_time, duration) == -1)
             goto done;
 
     rc = 0;
@@ -226,7 +213,7 @@ extern "C" planner_t *planner_multi_planner_at (planner_multi_t *ctx,
         errno = EINVAL;
         goto done;
     }
-    planner = ctx->plan_multi->get_planners_at (i);
+    planner = ctx->plan_multi->get_planner_at (i);
 done:
     return planner;
 }
@@ -249,19 +236,21 @@ extern "C" int64_t planner_multi_avail_time_first (
     fill_iter_request (ctx, &(ctx->plan_multi->get_iter ()),
                        on_or_after, duration, resource_requests, len);
 
-    if ((t = planner_avail_time_first (ctx->plan_multi->get_planners_at (0), on_or_after,
-                                       duration, resource_requests[0])) == -1)
+    if ((t = planner_avail_time_first (ctx->plan_multi->get_planner_at (
+                                            static_cast<size_t> (0)), on_or_after,
+                                            duration, resource_requests[0])) == -1)
         goto done;
 
     do {
         unmet = 0;
         for (i = 1; i < ctx->plan_multi->get_planners_size (); ++i) {
-            if ((unmet = planner_avail_during (ctx->plan_multi->get_planners_at (i),
+            if ((unmet = planner_avail_during (ctx->plan_multi->get_planner_at (i),
                                                t, duration,
                                                resource_requests[i])) == -1)
                 break;
         }
-    } while (unmet && (t = planner_avail_time_next (ctx->plan_multi->get_planners_at (0))) != -1);
+    } while (unmet && (t = planner_avail_time_next (
+                            ctx->plan_multi->get_planner_at (static_cast<size_t> (0)))) != -1);
 
 done:
     return t;
@@ -277,13 +266,13 @@ extern "C" int64_t planner_multi_avail_time_next (planner_multi_t *ctx)
         errno = EINVAL;
         goto done;
     }
-
     do {
         unmet = 0;
-        if ((t = planner_avail_time_next (ctx->plan_multi->get_planners_at (0))) == -1)
+        if ((t = planner_avail_time_next (
+                        ctx->plan_multi->get_planner_at (static_cast<size_t> (0)))) == -1)
             break;
         for (i = 1; i < ctx->plan_multi->get_planners_size (); ++i) {
-            if ((unmet = planner_avail_during (ctx->plan_multi->get_planners_at (i), t,
+            if ((unmet = planner_avail_during (ctx->plan_multi->get_planner_at (i), t,
                                                ctx->plan_multi->get_iter ().duration,
                                                ctx->plan_multi->get_iter ().counts[i])) == -1)
                 break;
@@ -301,7 +290,7 @@ extern "C" int64_t planner_multi_avail_resources_at (
         errno = EINVAL;
         return -1;
     }
-    return planner_avail_resources_at (ctx->plan_multi->get_planners_at (i), at);
+    return planner_avail_resources_at (ctx->plan_multi->get_planner_at (i), at);
 }
 
 extern "C" int planner_multi_avail_resources_array_at (
@@ -315,7 +304,7 @@ extern "C" int planner_multi_avail_resources_array_at (
         return -1;
     }
     for (i = 0; i < ctx->plan_multi->get_planners_size (); ++i) {
-        rc = planner_avail_resources_at (ctx->plan_multi->get_planners_at (i), at);
+        rc = planner_avail_resources_at (ctx->plan_multi->get_planner_at (i), at);
         if (rc == -1)
             break;
         resource_counts[i] = rc;
@@ -334,7 +323,7 @@ extern "C" int planner_multi_avail_during (
         return -1;
     }
     for (i = 0; i < ctx->plan_multi->get_planners_size (); ++i) {
-        rc = planner_avail_during (ctx->plan_multi->get_planners_at (i), at, duration,
+        rc = planner_avail_during (ctx->plan_multi->get_planner_at (i), at, duration,
                                    resource_requests[i]);
         if (rc == -1)
             break;
@@ -349,12 +338,13 @@ extern "C" int planner_multi_avail_resources_array_during (
     size_t i = 0;
     int64_t rc = 0;
     if (!ctx || !resource_counts
-        || ctx->plan_multi->get_planners_size () < 1 || ctx->plan_multi->get_planners_size () != len) {
+        || ctx->plan_multi->get_planners_size () < 1
+        || ctx->plan_multi->get_planners_size () != len) {
         errno = EINVAL;
         return -1;
     }
     for (i = 0; i < ctx->plan_multi->get_planners_size (); ++i) {
-        rc = planner_avail_resources_during (ctx->plan_multi->get_planners_at (i), at, duration);
+        rc = planner_avail_resources_during (ctx->plan_multi->get_planner_at (i), at, duration);
         if (rc == -1)
             break;
         resource_counts[i] = rc;
@@ -386,7 +376,7 @@ extern "C" int64_t planner_multi_add_span (
     ctx->plan_multi->incr_span_counter ();
 
     for (i = 0; i < len; ++i) {
-        if ( (span = planner_add_span (ctx->plan_multi->get_planners_at (i),
+        if ( (span = planner_add_span (ctx->plan_multi->get_planner_at (i),
                                        start_time, duration,
                                        resource_requests[i])) == -1) {
             ctx->plan_multi->get_span_lookup ().erase (mspan);
@@ -412,7 +402,7 @@ extern "C" int planner_multi_rem_span (planner_multi_t *ctx, int64_t span_id)
         goto done;
     }
     for (i = 0; i < it->second.size (); ++i) {
-        if (planner_rem_span (ctx->plan_multi->get_planners_at (i), it->second[i]) == -1)
+        if (planner_rem_span (ctx->plan_multi->get_planner_at (i), it->second[i]) == -1)
             goto done;
     }
     ctx->plan_multi->get_span_lookup ().erase (it);
diff --git a/resource/traversers/dfu_impl.hpp b/resource/traversers/dfu_impl.hpp
index 23048c01a..7b7b11c48 100644
--- a/resource/traversers/dfu_impl.hpp
+++ b/resource/traversers/dfu_impl.hpp
@@ -510,10 +510,11 @@ int dfu_impl_t::count_relevant_types (planner_multi_t *plan,
 {
     int rc = 0;
     size_t len = planner_multi_resources_len (plan);
-    const char **resource_types = planner_multi_resource_types (plan);
+    const char *type = nullptr;
     for (unsigned int i = 0; i < len; ++i) {
-        if (lookup.find (resource_types[i]) != lookup.end ()) {
-            uint64_t n = (uint64_t)lookup.at (resource_types[i]);
+        type = planner_multi_resource_type_at (plan, i);
+        if (lookup.find (type) != lookup.end ()) {
+            uint64_t n = (uint64_t)lookup.at (type);
             resource_counts.push_back (n);
         } else {
             resource_counts.push_back (0);

From ccf5abc41e475a93fefdbc62c1ef6eff2dfd6308 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Wed, 10 Jan 2024 11:58:56 -0800
Subject: [PATCH 09/17] planner: add update functionality

Problem: when updating the Fluxion resource graph the `planner`
scheduled points' remaining resource count must be updated.

Add an `update` function to add or subtract from the current remaining
resource count at each scheduled point. Create a guard to ensure the
remaining count is nonnegative.
---
 resource/planner/c++/planner.cpp           | 24 ++++++++++++++++++++++
 resource/planner/c++/planner.hpp           |  1 +
 resource/planner/c/planner.h               | 13 ++++++++++++
 resource/planner/c/planner_c_interface.cpp |  6 ++++++
 4 files changed, 44 insertions(+)

diff --git a/resource/planner/c++/planner.cpp b/resource/planner/c++/planner.cpp
index 0f55c0dc5..4d6963daa 100644
--- a/resource/planner/c++/planner.cpp
+++ b/resource/planner/c++/planner.cpp
@@ -229,6 +229,30 @@ int planner::restore_track_points ()
     return rc;
 }
 
+int planner::update_total (uint64_t resource_total)
+{
+    int64_t delta = resource_total - m_total_resources;
+    int64_t tmp = 0;
+    scheduled_point_t *point = nullptr;
+    if (delta == 0)
+        return 0;
+    m_total_resources = static_cast<int64_t> (resource_total);
+    point = m_sched_point_tree.get_state (m_plan_start);
+    while (point) {
+        // Prevent remaining from taking negative values. This should 
+        // reduce likelihood of errors when adding and removing spans.
+        // If the performance penalty is non-negligible we can 
+        // investigate letting remaining take negative values.
+        tmp = point->remaining + delta;
+        if (tmp >= 0)
+            point->remaining = tmp;
+        else
+            point->remaining = 0;
+        point = m_sched_point_tree.next (point);
+    }
+    return 0;
+}
+
 int64_t planner::get_total_resources () const
 {
     return m_total_resources;
diff --git a/resource/planner/c++/planner.hpp b/resource/planner/c++/planner.hpp
index 05fc39f75..6d3fceb94 100644
--- a/resource/planner/c++/planner.hpp
+++ b/resource/planner/c++/planner.hpp
@@ -51,6 +51,7 @@ class planner {
     int erase ();
     int reinitialize (int64_t base_time, uint64_t duration);
     int restore_track_points ();
+    int update_total (uint64_t resource_total);
     
     // Resources and duration
     int64_t get_total_resources () const;
diff --git a/resource/planner/c/planner.h b/resource/planner/c/planner.h
index 0f777973f..a72aefa13 100644
--- a/resource/planner/c/planner.h
+++ b/resource/planner/c/planner.h
@@ -229,6 +229,19 @@ int64_t planner_span_resource_count (planner_t *ctx, int64_t span_id);
 */
 bool planners_equal (planner_t *lhs, planner_t *rhs);
 
+/*! Update the resource count to support elasticity.
+ *
+ *  \param ctx          opaque planner context returned from planner_new.
+ *  \param resource_total
+ *                      64-bit unsigned integer of
+ *                      the total count of available resources
+ *                      of the resource type.
+ *  \return             0 on success; -1 on an error with errno set as follows:
+ *                          EINVAL: invalid argument.
+ */
+int planner_update_total (planner_t *ctx,
+                          uint64_t resource_total);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/resource/planner/c/planner_c_interface.cpp b/resource/planner/c/planner_c_interface.cpp
index e0f154f9c..0d7c0e5c1 100644
--- a/resource/planner/c/planner_c_interface.cpp
+++ b/resource/planner/c/planner_c_interface.cpp
@@ -698,6 +698,12 @@ extern "C" bool planners_equal (planner_t *lhs, planner_t *rhs)
     return (*(lhs->plan) == *(rhs->plan));
 }
 
+extern "C" int planner_update_total (planner_t *ctx,
+                                     uint64_t resource_total)
+{
+    return ctx->plan->update_total (resource_total);
+}
+
 /*
  * vi: ts=4 sw=4 expandtab
  */

From f9e78b3f570322f141719d5381f98facd3fd038a Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Wed, 10 Jan 2024 17:27:34 -0800
Subject: [PATCH 10/17] planner_multi: substitute map to track request counts

Problem: when updating the Fluxion resource graph `request_multi`
`counts`' indices can be reordered. Reordering will cause inconsistent
lookups.

Update the `request_multi` `counts` to be map-based to handle reordering
of indices.
---
 resource/planner/c++/planner_multi.cpp           | 6 +++---
 resource/planner/c++/planner_multi.hpp           | 3 ++-
 resource/planner/c/planner_multi_c_interface.cpp | 7 +++++--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/resource/planner/c++/planner_multi.cpp b/resource/planner/c++/planner_multi.cpp
index 9753a2658..db462948b 100644
--- a/resource/planner/c++/planner_multi.cpp
+++ b/resource/planner/c++/planner_multi.cpp
@@ -50,7 +50,7 @@ planner_multi::planner_multi (int64_t base_time, uint64_t duration,
            errno = ENOMEM;
            throw std::bad_alloc ();
         }
-        m_iter.counts.push_back (0);
+        m_iter.counts[type] = 0;
         m_types_totals_planners.push_back ({type, resource_totals[i], p});
     }
     m_span_counter = 0;
@@ -82,8 +82,8 @@ planner_multi::planner_multi (const planner_multi &o)
         m_types_totals_planners.push_back ({iter.resource_type,
                                             iter.resource_total, np});
     }
-    m_span_lookup = o.m_span_lookup;
     m_iter = o.m_iter;
+    m_span_lookup = o.m_span_lookup;
     m_span_lookup_iter = o.m_span_lookup_iter;
     m_span_counter = o.m_span_counter;
 }
@@ -119,8 +119,8 @@ planner_multi &planner_multi::operator= (const planner_multi &o)
         m_types_totals_planners.push_back ({iter.resource_type,
                                             iter.resource_total, np});
     }
-    m_span_lookup = o.m_span_lookup;
     m_iter = o.m_iter;
+    m_span_lookup = o.m_span_lookup;
     m_span_lookup_iter = o.m_span_lookup_iter;
     m_span_counter = o.m_span_counter;
 
diff --git a/resource/planner/c++/planner_multi.hpp b/resource/planner/c++/planner_multi.hpp
index 4883e2256..4880c45cb 100644
--- a/resource/planner/c++/planner_multi.hpp
+++ b/resource/planner/c++/planner_multi.hpp
@@ -12,6 +12,7 @@
 #define PLANNER_MULTI_HPP
 
 #include "planner.hpp"
+#include <unordered_map>
 #include <boost/multi_index_container.hpp>
 #include <boost/multi_index/member.hpp>
 #include <boost/multi_index/random_access_index.hpp>
@@ -20,7 +21,7 @@
 struct request_multi {
     int64_t on_or_after = 0;
     uint64_t duration = 0;
-    std::vector<int64_t> counts;
+    std::unordered_map<std::string, int64_t> counts;
 };
 
 struct planner_multi_meta {
diff --git a/resource/planner/c/planner_multi_c_interface.cpp b/resource/planner/c/planner_multi_c_interface.cpp
index 3c6a07482..4421eb779 100644
--- a/resource/planner/c/planner_multi_c_interface.cpp
+++ b/resource/planner/c/planner_multi_c_interface.cpp
@@ -32,7 +32,8 @@ static void fill_iter_request (planner_multi_t *ctx, struct request_multi *iter,
     iter->on_or_after = at;
     iter->duration = duration;
     for (i = 0; i < len; ++i)
-        iter->counts[i] = resources[i];
+        iter->counts[ctx->plan_multi->get_resource_type_at (i)] =
+                resources[i];
 }
 
 extern "C" planner_multi_t *planner_multi_new (
@@ -261,6 +262,7 @@ extern "C" int64_t planner_multi_avail_time_next (planner_multi_t *ctx)
     size_t i = 0;
     int unmet = 0;
     int64_t t = -1;
+    std::string type;
 
     if (!ctx) {
         errno = EINVAL;
@@ -272,9 +274,10 @@ extern "C" int64_t planner_multi_avail_time_next (planner_multi_t *ctx)
                         ctx->plan_multi->get_planner_at (static_cast<size_t> (0)))) == -1)
             break;
         for (i = 1; i < ctx->plan_multi->get_planners_size (); ++i) {
+            type = ctx->plan_multi->get_resource_type_at (i);
             if ((unmet = planner_avail_during (ctx->plan_multi->get_planner_at (i), t,
                                                ctx->plan_multi->get_iter ().duration,
-                                               ctx->plan_multi->get_iter ().counts[i])) == -1)
+                                               ctx->plan_multi->get_iter ().counts.at (type))) == -1)
                 break;
         }
     } while (unmet);

From bcceb1f671abe1d76d2c8e23bd982b1733047359 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Wed, 10 Jan 2024 17:06:16 -0800
Subject: [PATCH 11/17] planner_multi: add update functionality

Problem: when updating the Fluxion resource graph planners, resources,
counts, and their orders can be changed by mutation of the resource
graph topology and attributes.

Add `update` functionality to `planner_multi` and the C interface to add
new resources and planners, reorder, modify, and permute existing
resources, and delete removed resources.
---
 resource/planner/c++/planner_multi.cpp        | 68 +++++++++++++++++++
 resource/planner/c++/planner_multi.hpp        | 10 +++
 resource/planner/c/planner_multi.h            | 22 ++++++
 .../planner/c/planner_multi_c_interface.cpp   | 58 ++++++++++++++++
 4 files changed, 158 insertions(+)

diff --git a/resource/planner/c++/planner_multi.cpp b/resource/planner/c++/planner_multi.cpp
index db462948b..f3ad657b2 100644
--- a/resource/planner/c++/planner_multi.cpp
+++ b/resource/planner/c++/planner_multi.cpp
@@ -182,6 +182,48 @@ planner_multi::~planner_multi ()
     erase ();
 }
 
+void planner_multi::add_planner (int64_t base_time, uint64_t duration,
+                                 const uint64_t resource_total,
+                                 const char *resource_type, size_t i)
+{
+    std::string type;
+    planner_t *p = nullptr;
+
+    try {
+        type = std::string (resource_type);
+        p = new planner_t (base_time, duration,
+                           resource_total,
+                           resource_type);
+    } catch (std::bad_alloc &e) {
+        errno = ENOMEM;
+        throw std::bad_alloc ();
+    }
+    m_iter.counts[type] = 0;
+    if (i > m_types_totals_planners.size ())
+        m_types_totals_planners.push_back ({type, resource_total, p});
+    else {
+        auto it = m_types_totals_planners.begin () + i;
+        m_types_totals_planners.insert (
+                        it, planner_multi_meta{type, resource_total, p});
+    }
+    
+}
+
+void planner_multi::delete_planners (const std::unordered_set<std::string> &rtypes)
+{
+    auto &by_res = m_types_totals_planners.get<res_type> ();
+    for (auto iter = by_res.begin (); iter != by_res.end ();) {
+        if (rtypes.find (iter->resource_type) == rtypes.end ()) {
+            // need to remove from request_multi
+            m_iter.counts.erase (iter->resource_type);
+            // Trigger planner destructor
+            delete iter->planner;
+            iter = by_res.erase (iter);
+        } else
+            ++iter;
+    }
+}
+
 planner_t *planner_multi::get_planner_at (size_t i) const
 {
     return m_types_totals_planners.at (i).planner;
@@ -193,6 +235,32 @@ planner_t *planner_multi::get_planner_at (const char *type) const
     return by_res.find (std::string (type))->planner;
 }
 
+void planner_multi::update_planner_index (const char *type, size_t i)
+{
+    std::string rtype = std::string (type);
+    auto by_res = m_types_totals_planners.get<res_type> ().find (rtype);
+    auto new_idx = m_types_totals_planners.begin () + i;
+    auto curr_idx = m_types_totals_planners.get<idx> ().iterator_to (*by_res);
+    // noop if new_idx == curr_idx
+    m_types_totals_planners.relocate (new_idx, curr_idx);
+}
+
+int planner_multi::update_planner_total (uint64_t total, size_t i)
+{
+    m_types_totals_planners.at (i).resource_total = total;
+    return m_types_totals_planners.at (i).planner->plan->update_total (total);
+}
+
+bool planner_multi::planner_at (const char *type) const
+{
+    auto &by_res = m_types_totals_planners.get<res_type> ();
+    auto result = by_res.find (std::string (type));
+    if (result == by_res.end ())
+        return false;
+    else
+        return true;
+}
+
 size_t planner_multi::get_planners_size () const
 {
     return m_types_totals_planners.size ();
diff --git a/resource/planner/c++/planner_multi.hpp b/resource/planner/c++/planner_multi.hpp
index 4880c45cb..a2bb66069 100644
--- a/resource/planner/c++/planner_multi.hpp
+++ b/resource/planner/c++/planner_multi.hpp
@@ -13,6 +13,7 @@
 
 #include "planner.hpp"
 #include <unordered_map>
+#include <unordered_set>
 #include <boost/multi_index_container.hpp>
 #include <boost/multi_index/member.hpp>
 #include <boost/multi_index/random_access_index.hpp>
@@ -65,6 +66,9 @@ class planner_multi {
     // Public getters and setters
     planner_t *get_planner_at (size_t i) const;
     planner_t *get_planner_at (const char *type) const;
+    void update_planner_index (const char *type, size_t i);
+    int update_planner_total (uint64_t total, size_t i);
+    bool planner_at (const char *type) const;
     size_t get_planners_size () const;
     int64_t get_resource_total_at (size_t i) const;
     int64_t get_resource_total_at (const char *type) const;
@@ -81,6 +85,12 @@ class planner_multi {
     uint64_t get_span_counter ();
     void set_span_counter (uint64_t sc);
     void incr_span_counter ();
+    void add_planner (int64_t base_time, uint64_t duration,
+                      const uint64_t resource_total,
+                      const char *resource_type, size_t i);
+    // Assuming small number of resources,
+    // could try set, too
+    void delete_planners (const std::unordered_set<std::string> &rtypes);
 
 private:
     multi_container m_types_totals_planners;
diff --git a/resource/planner/c/planner_multi.h b/resource/planner/c/planner_multi.h
index 2dad4f4ba..8b2d1de40 100644
--- a/resource/planner/c/planner_multi.h
+++ b/resource/planner/c/planner_multi.h
@@ -292,6 +292,28 @@ size_t planner_multi_span_size (planner_multi_t *ctx);
 */
 bool planner_multis_equal (planner_multi_t *lhs, planner_multi_t *rhs);
 
+/*! Update the counts and resource types to support elasticity.
+ *
+ *  \param ctx          opaque multi-planner context returned
+ *                      from planner_multi_new.
+ *  \param resource_totals
+ *                      64-bit unsigned integer array of size len where each
+ *                      element contains the total count of available resources
+ *                      of a single resource type.
+ *  \param resource_types
+ *                      string array of size len where each element contains
+ *                      the resource type corresponding to each corresponding
+ *                      element in the resource_totals array.
+ *  \param len          length of resource_counts and resource_types arrays.
+ *  \return             0 on success; -1 on an error with errno set as follows:
+ *                          EINVAL: invalid argument.
+ */
+int planner_multi_update (planner_multi_t *ctx,
+                          const uint64_t *resource_totals,
+                          const char **resource_types,
+                          size_t len);
+
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/resource/planner/c/planner_multi_c_interface.cpp b/resource/planner/c/planner_multi_c_interface.cpp
index 4421eb779..18b3cb0ca 100644
--- a/resource/planner/c/planner_multi_c_interface.cpp
+++ b/resource/planner/c/planner_multi_c_interface.cpp
@@ -14,6 +14,7 @@
 #include <limits>
 #include <vector>
 #include <map>
+#include <unordered_set>
 
 #include "planner_multi.h"
 #include "resource/planner/c++/planner_multi.hpp"
@@ -466,6 +467,63 @@ extern "C" bool planner_multis_equal (planner_multi_t *lhs,
     return (*(lhs->plan_multi) == *(rhs->plan_multi));
 }
 
+extern "C" int planner_multi_update (planner_multi_t *ctx,
+                                     const uint64_t *resource_totals,
+                                     const char **resource_types,
+                                     size_t len)
+{
+    int rc = -1;
+    size_t i = 0;
+    // Assuming small number of resource types,
+    // could try set, too
+    std::unordered_set<std::string> rtypes;
+    int64_t base_time = 0;
+    int64_t duration = 0;
+
+    if (!ctx || !resource_totals || !resource_types) {
+        errno = EINVAL;
+        goto done;
+    }
+    base_time = planner_base_time (
+                    ctx->plan_multi->get_planner_at (static_cast<size_t> (0)));
+    duration = planner_duration (
+                    ctx->plan_multi->get_planner_at (static_cast<size_t> (0)));
+    if (duration < 0) {
+        errno = EINVAL;
+        goto done;
+    }
+
+    for (i = 0; i < len; ++i) {
+        if (resource_totals[i] >
+              static_cast<uint64_t> (std::numeric_limits<int64_t>::max ())) {
+                errno = ERANGE;
+                goto done;
+        }
+        rtypes.insert (resource_types[i]);
+        if (!ctx->plan_multi->planner_at (resource_types[i])) {
+            // Assume base_time same as parent
+            ctx->plan_multi->add_planner (base_time, static_cast<uint64_t> (duration),
+                                          resource_totals[i], resource_types[i], i);
+        } else {
+            // Index could have changed
+            ctx->plan_multi->update_planner_index (resource_types[i], i);
+            if ( (rc = ctx->plan_multi->update_planner_total (resource_totals[i],
+                                                              i)) != 0) {
+                errno = EINVAL;
+                goto done;
+            }
+        }
+    }
+    // remove values not in new types
+    if (rtypes.size () > 0)
+        ctx->plan_multi->delete_planners (rtypes);
+
+    rc = 0;
+
+done:
+    return rc;
+}
+
 /*
  * vi: ts=4 sw=4 expandtab
  */

From a475d5e4f16832c3743ef9ad5b39749c5ac9b635 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Sun, 7 Jan 2024 16:47:46 -0800
Subject: [PATCH 12/17] testsuite: add unit tests for planner update

Problem: updating a `planner` requires unit tests to maintain correct
functionality.

Add update unit tests for `planner`.
---
 resource/planner/test/planner_test01.cpp | 57 +++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/resource/planner/test/planner_test01.cpp b/resource/planner/test/planner_test01.cpp
index 97793d715..3a50c998d 100644
--- a/resource/planner/test/planner_test01.cpp
+++ b/resource/planner/test/planner_test01.cpp
@@ -723,9 +723,62 @@ static int test_constructors_and_overload ()
     return 0;
 }
 
+static int test_update ()
+{
+    int rc;
+    int64_t span;
+    bool bo = false;
+    uint64_t resource_total = 100000;
+    uint64_t resource1 = 36;
+    uint64_t resource2 = 3600;
+    uint64_t resource3 = 1800;
+    uint64_t resource4 = 2304;
+    uint64_t resource5 = 468;
+    uint64_t resource6 = 50000;
+    int64_t avail, avail1 = 0;
+    const char resource_type[] = "core";
+    planner_t *ctx, *ctx2 = NULL;
+
+    ctx = planner_new (0, INT64_MAX, resource_total, resource_type);
+    // Add some spans
+    planner_add_span (ctx, 0, 600, resource1);
+    planner_add_span (ctx, 0, 57600, resource2);
+    planner_add_span (ctx, 57600, 57600, resource3);
+    planner_add_span (ctx, 172800, 57600, resource4);
+    planner_add_span (ctx, 115200, 900, resource5);
+
+    avail = planner_avail_resources_at (ctx, 0);
+
+    ctx2 = planner_copy (ctx);
+
+    rc = planner_update_total (ctx, 100000);
+
+    bo = (bo || !(planners_equal (ctx, ctx)));
+    ok (!bo, "update with same resource count shouldn't change planner");
+
+    rc = planner_update_total (ctx, 50000);
+    avail1 = planner_avail_resources_at (ctx, 0);
+    bo = (bo || !(planners_equal (ctx, ctx)));
+    ok (!bo, "avail difference for valid reduction is correct");
+
+    rc = planner_update_total (ctx, 100000);
+    bo = (bo || !(planners_equal (ctx, ctx2)));
+    ok (!bo, "re-adding resources shouldn't change planner");
+
+    rc = planner_update_total (ctx, 40000);
+    span = planner_add_span (ctx, 1152000, 57600, resource6);
+    bo = (bo || (planners_equal (ctx, ctx2)) || span != -1);
+    ok (!bo, "reducing resources below request should prevent scheduling");
+
+    planner_destroy (&ctx);
+    planner_destroy (&ctx2);
+
+    return 0;
+}
+
 int main (int argc, char *argv[])
 {
-    plan (63);
+    plan (67);
 
     test_planner_getters ();
 
@@ -749,6 +802,8 @@ int main (int argc, char *argv[])
 
     test_constructors_and_overload ();
 
+    test_update ();
+
     done_testing ();
 
     return EXIT_SUCCESS;

From 7ca464925c6279578dd917a903e076f24dec7647 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Tue, 20 Feb 2024 20:16:14 -0800
Subject: [PATCH 13/17] testsuite: add unit tests for planner_multi update

Problem: updating a `planner_multi` requires unit tests to maintain
correct functionality.

Add update unit tests for `planner_multi`.
---
 resource/planner/test/planner_test02.cpp | 81 +++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 1 deletion(-)

diff --git a/resource/planner/test/planner_test02.cpp b/resource/planner/test/planner_test02.cpp
index dd4db132d..347a0146c 100644
--- a/resource/planner/test/planner_test02.cpp
+++ b/resource/planner/test/planner_test02.cpp
@@ -615,10 +615,87 @@ static int test_constructors_and_overload ()
 
 }
 
+static int test_multi_update ()
+{
+    bool bo = false, found = true;
+    size_t len = 5;
+    size_t size = 0;
+    int rc = -1;
+    int64_t span = -1, avail = -1, avail1 = -1, total = -1;
+    const uint64_t resource_totals[] = {10, 20, 30, 40, 50};
+    const uint64_t resource_totals1[] = {5, 10, 20};
+    const uint64_t resource_totals2[] = {10, 20, 30, 40, 50, 60, 70};
+    const char *resource_types[] = {"A", "B", "C", "D", "E"};
+    const char *resource_types1[] = {"B", "C", "D"};
+    const char *resource_types2[] = {"B", "A", "C", "D", "G", "X", "Y"};
+    const uint64_t request1[] = {1, 0, 0, 0, 0};
+    const uint64_t request2[] = {0, 2, 0, 0, 0};
+    const uint64_t request3[] = {0, 0, 3, 0, 0};
+    const uint64_t request4[] = {0, 0, 20};
+    const uint64_t request5[] = {0, 0, 21};
+    const uint64_t request6[] = {10, 20, 30, 40, 50, 60, 70};
+    planner_multi_t *ctx = NULL, *ctx2 = NULL;
+
+    ctx = planner_multi_new (0, INT64_MAX, resource_totals, resource_types, len);
+
+    planner_multi_add_span (ctx, 0, 1000, request1, len);
+    span = planner_multi_add_span (ctx, 1000, 1000, request2, len);
+    planner_multi_add_span (ctx, 2000, 1000, request3, len);
+    avail = planner_multi_avail_resources_at (ctx, 0, 0);
+
+    ctx2 = planner_multi_copy (ctx);
+
+    rc = planner_multi_update (ctx, resource_totals, resource_types, len);
+    avail1 = planner_multi_avail_resources_at (ctx, 0, 0);
+    bo = (bo || !(planner_multis_equal (ctx, ctx2)) || avail != avail1 || rc != 0);
+    ok (!bo, "update with same resource count shouldn't change planner_multi");
+
+    rc = planner_multi_update (ctx, resource_totals1, resource_types1, 3);
+    avail1 = planner_multi_avail_resources_at (ctx, 1000, 0);
+    bo = (bo || (planner_multis_equal (ctx, ctx2)) || avail1 != 3 || rc != 0);
+    ok (!bo, "resource reduction results in expected resources and availability");
+
+    rc = planner_multi_update (ctx, resource_totals, resource_types, len);
+    bo = (bo || (planner_multis_equal (ctx, ctx2)) || rc != 0);
+    ok (!bo, "re-adding resources can't restore planner_multi state after removal");
+
+    rc = planner_multi_update (ctx, resource_totals1, resource_types1, 3);
+    span = planner_multi_add_span (ctx, 2000, 1000, request4, 3);
+    avail1 = planner_multi_avail_resources_at (ctx, 2000, 2);
+    bo = (bo || avail1 != 0 || span == -1 || rc != 0);
+    ok (!bo, "can allocate full updated resources");
+
+    span = planner_multi_add_span (ctx, 3000, 1000, request5, 3);
+    avail1 = planner_multi_avail_resources_at (ctx, 3000, 2);
+    bo = (bo || avail1 != 20 || span != -1 || rc != 0);
+    ok (!bo, "can't overallocate updated resources");
+
+    rc = planner_multi_update (ctx, resource_totals2, resource_types2, 7);
+    span = planner_multi_add_span (ctx, 4000, 1000, request6, 7);
+    avail1 = planner_multi_avail_resources_at (ctx, 4000, 6);
+    bo = (bo || avail1 != 0 || span == -1 || rc != 0);
+    ok (!bo, "can allocate full added resources");
+
+    for (int i = 0; i < planner_multi_resources_len (ctx); ++i) {
+        if (planner_multi_resource_total_by_type (ctx, resource_types2[i])
+                    != resource_totals2[i]) {
+            found = false;
+            break;
+        }
+    }
+    bo = (bo || !found);
+    ok (!bo, "can look up resources by string");
+
+    planner_multi_destroy (&ctx);
+    planner_multi_destroy (&ctx2);
+
+    return 0;
+
+}
 
 int main (int argc, char *argv[])
 {
-    plan (91);
+    plan (98);
 
     test_multi_basics ();
 
@@ -634,6 +711,8 @@ int main (int argc, char *argv[])
 
     test_constructors_and_overload ();
 
+    test_multi_update ();
+
     done_testing ();
 
     return EXIT_SUCCESS;

From 0638380ceb37b7825163a24221a37cf57017296e Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Wed, 31 Jan 2024 00:17:26 -0800
Subject: [PATCH 14/17] testsuite: enable valgrind tests and test planner and
 schema

Problem: the migration to cmake broke the ability to run Valngrind
tests, and Valgrind doesn't detect leaks in important Fluxion
components when running under Flux.

Add capability to find Valgrind and run Valgrind tests. Add additional
tests for planner and schema.
---
 CMakeLists.txt           |  2 ++
 cmake/FindValgrind.cmake | 19 +++++++++++++++++
 t/t5000-valgrind.t       | 44 +++++++++++++++++++++++++++++++++++++---
 3 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 cmake/FindValgrind.cmake

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a2a32897d..82c9c4690 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -172,6 +172,8 @@ if(DEFINED ENV{WITH_GO})
   include(GolangSimple)
 endif()
 
+include(FindValgrind) # Search for Valgrind
+
 include_directories(.)
 add_subdirectory( etc )
 add_subdirectory( src )
diff --git a/cmake/FindValgrind.cmake b/cmake/FindValgrind.cmake
new file mode 100644
index 000000000..74062e8d8
--- /dev/null
+++ b/cmake/FindValgrind.cmake
@@ -0,0 +1,19 @@
+# Look for Valgrind headers and binary.
+#
+# Variables defined by this module:
+# 	Valgrind_FOUND System has valgrind
+# 	Valgrind_INCLUDE_DIR where to find valgrind/memcheck.h, etc.
+# 	Valgrind_EXECUTABLE the valgrind executable.
+# This module appends to config.h so t5000-valgrind.t succeeds.
+# We may need to change this behavior once remaining autotools 
+# files are removed.
+
+find_path(Valgrind_INCLUDE_DIR valgrind HINTS ${Valgrind_INCLUDE_PATH})
+find_program(Valgrind_EXECUTABLE NAMES valgrind PATH ${Valgrind_BINARY_PATH})
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(Valgrind DEFAULT_MSG Valgrind_INCLUDE_DIR Valgrind_EXECUTABLE)
+
+if(Valgrind_FOUND)
+	file(APPEND config.h "#define HAVE_VALGRIND 1\n")
+endif()
diff --git a/t/t5000-valgrind.t b/t/t5000-valgrind.t
index 34198fb97..15c793c5b 100755
--- a/t/t5000-valgrind.t
+++ b/t/t5000-valgrind.t
@@ -7,7 +7,7 @@ test -n "$FLUX_TESTS_LOGFILE" && set -- "$@" --logfile
 . `dirname $0`/sharness.sh
 
 #  Do not run valgrind test by default unless FLUX_ENABLE_VALGRIND_TEST
-#   is set in environment (e.g. by CI), or the test run run with -d, --debug
+#  is set in environment (e.g. by CI), or the test run run with -d, --debug
 #
 if test -z "$FLUX_ENABLE_VALGRIND_TEST" && test "$debug" = ""; then
     skip_all='skipping valgrind tests since FLUX_ENABLE_VALGRIND_TEST not set'
@@ -23,7 +23,7 @@ if ! test_have_prereq NO_ASAN; then
     test_done
 fi
 
-# Do not run test by default unless valgrind/valgrind.h was found, since
+#  Do not run test by default unless valgrind/valgrind.h was found, since
 #  this has been known to introduce false positives (#1097). However, allow
 #  run to be forced on the cmdline with -d, --debug.
 #
@@ -48,7 +48,7 @@ test_expect_success \
 	run_timeout 900 \
 	flux start -s ${VALGRIND_NBROKERS} \
 		--killer-timeout=120 \
-		--wrap=libtool,e,${VALGRIND} \
+		--wrap=${VALGRIND} \
 		--wrap=--tool=memcheck \
 		--wrap=--leak-check=full \
 		--wrap=--gen-suppressions=all \
@@ -60,4 +60,42 @@ test_expect_success \
 		--wrap=--suppressions=$VALGRIND_SUPPRESSIONS \
 		 ${VALGRIND_WORKLOAD}
 '
+
+# The Valgrind test above doesn't detect memory leaks in planner or schema
+test_expect_success \
+  "valgrind reports no new errors on planner test 01" '
+		${VALGRIND} \
+		--tool=memcheck \
+		--leak-check=full \
+		--error-exitcode=1 \
+		${SHARNESS_BUILD_DIRECTORY}/resource/planner/test/planner_test01
+'
+
+test_expect_success \
+  "valgrind reports no new errors on planner test 02" '
+		${VALGRIND} \
+		--tool=memcheck \
+		--leak-check=full \
+		--error-exitcode=1 \
+		${SHARNESS_BUILD_DIRECTORY}/resource/planner/test/planner_test02
+'
+
+test_expect_success \
+  "valgrind reports no new errors on schema test 01" '
+		${VALGRIND} \
+		--tool=memcheck \
+		--leak-check=full \
+		--error-exitcode=1 \
+		${SHARNESS_BUILD_DIRECTORY}/resource/schema/test/schema_test01
+'
+
+test_expect_success \
+  "valgrind reports no new errors on schema test 02" '
+		${VALGRIND} \
+		--tool=memcheck \
+		--leak-check=full \
+		--error-exitcode=1 \
+		${SHARNESS_BUILD_DIRECTORY}/resource/schema/test/schema_test02
+'
+
 test_done

From e0b191299e610e56a5943c1ce7572edac152ad8b Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Tue, 20 Feb 2024 19:26:15 -0800
Subject: [PATCH 15/17] testsuite: fix memory leaks in planner tests

Problem: the current planner tests do not destroy the planners created
during the tests, resulting in memory leaks that will pollute Valgrind
test output.

Add calls to destroy the planners to eliminate the memory leaks.
---
 resource/planner/test/planner_test01.cpp | 5 +++++
 resource/planner/test/planner_test02.cpp | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/resource/planner/test/planner_test01.cpp b/resource/planner/test/planner_test01.cpp
index 3a50c998d..82473a802 100644
--- a/resource/planner/test/planner_test01.cpp
+++ b/resource/planner/test/planner_test01.cpp
@@ -569,6 +569,9 @@ static int test_resource_service_flow ()
     }
     ok (!bo, "reserve %d jobs for global/local planners", depth);
 
+    for (auto &type : global_types)
+        free ((void *)type);
+
     planner_destroy (&global1);
     planner_destroy (&global2);
     planner_destroy (&global3);
@@ -639,6 +642,8 @@ static int test_more_add_remove ()
     bo = (bo || span6 == -1);
 
     ok (!bo, "more add-remove-add test works");
+
+    planner_destroy (&ctx);
     return 0;
 }
 
diff --git a/resource/planner/test/planner_test02.cpp b/resource/planner/test/planner_test02.cpp
index 347a0146c..c5ac053cc 100644
--- a/resource/planner/test/planner_test02.cpp
+++ b/resource/planner/test/planner_test02.cpp
@@ -295,6 +295,7 @@ static int test_multi_strictly_larger ()
     ok ((t == -1 && errno == ENOENT), "avail_time_next for (%s)",
          ss.str ().c_str ());
 
+    planner_multi_destroy (&ctx);
     return 0;
 }
 
@@ -405,6 +406,7 @@ static int test_multi_partially_larger ()
     ok ((t == -1 && errno == ENOENT), "avail_time_next for (%s)",
          ss.str ().c_str ());
 
+    planner_multi_destroy (&ctx);
     return 0;
 }
 
@@ -483,6 +485,8 @@ static int test_multi_many_spans ()
     t = planner_multi_avail_time_next (ctx);
     bo = (bo || !(t == -1 && errno == ENOENT));
     ok (!bo, "avail_time_next for (%s)", ss.str ().c_str ());
+
+    planner_multi_destroy (&ctx);
     return 0;
 }
 
@@ -535,6 +539,7 @@ static int test_multi_add_remove ()
     size = planner_multi_span_size (ctx);
     ok ((size == 2), "planner_multi_span_size works");
 
+    planner_multi_destroy (&ctx);
     return 0;
 
 }

From 9bad4b34b97e2455f2f13b3ce39515c1726b82bf Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Sat, 24 Feb 2024 18:58:13 -0800
Subject: [PATCH 16/17] testsuite: fix variable declaration for planner_multi
 unit tests

Problem: planner_multi_t contexts are declared and initialized using a
comma-separated list that can result in UB.

Correct the declaration and initialization.
---
 resource/planner/test/planner_test02.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/resource/planner/test/planner_test02.cpp b/resource/planner/test/planner_test02.cpp
index c5ac053cc..76b42a420 100644
--- a/resource/planner/test/planner_test02.cpp
+++ b/resource/planner/test/planner_test02.cpp
@@ -556,9 +556,10 @@ static int test_constructors_and_overload ()
     const uint64_t request1[] = {1, 0, 0, 0, 0};
     const uint64_t request2[] = {0, 2, 0, 0, 0};
     const uint64_t request3[] = {0, 0, 3, 0, 0};
-    planner_multi_t *ctx, *ctx2, *ctx3, *ctx4 = NULL;
+    planner_multi_t *ctx = NULL, *ctx2 = NULL, *ctx3 = NULL, *ctx4 = NULL;
 
-    ctx = planner_multi_new (0, INT64_MAX, resource_totals, resource_types, len);
+    ctx = planner_multi_new (0, INT64_MAX, resource_totals, resource_types,
+                             len);
 
     planner_multi_add_span (ctx, 0, 1000, request1, len);
     span = planner_multi_add_span (ctx, 1000, 1000, request2, len);

From 6f16387593e877fd25850bed54974850f7cbc546 Mon Sep 17 00:00:00 2001
From: Daniel Milroy <daniel.milroy@gmail.com>
Date: Sat, 24 Feb 2024 19:06:21 -0800
Subject: [PATCH 17/17] testsuite: fix variable declarations for planner unit
 tests

Problem: planner_multi_t contexts and ints are declared and initialized
using a comma-separated list that can result in UB.

Correct the declarations and initializations.
---
 resource/planner/test/planner_test01.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/resource/planner/test/planner_test01.cpp b/resource/planner/test/planner_test01.cpp
index 82473a802..c7077c485 100644
--- a/resource/planner/test/planner_test01.cpp
+++ b/resource/planner/test/planner_test01.cpp
@@ -584,7 +584,8 @@ static int test_resource_service_flow ()
 static int test_more_add_remove ()
 {
     int rc;
-    int64_t span1, span2, span3, span4, span5, span6;
+    int64_t span1 = -1, span2 = -1, span3 = -1, span4 = -1, span5 = -1,
+            span6 = -1;
     bool bo = false;
     uint64_t resource_total = 100000;
     uint64_t resource1 = 36;
@@ -660,7 +661,7 @@ static int test_constructors_and_overload ()
     uint64_t resource5 = 2304;
     uint64_t resource6 = 468;
     const char resource_type[] = "core";
-    planner_t *ctx, *ctx2, *ctx3, *ctx4 = NULL;
+    planner_t *ctx = NULL, *ctx2 = NULL, *ctx3 = NULL, *ctx4 = NULL;
 
     ctx = planner_new (0, INT64_MAX, resource_total, resource_type);
 
@@ -742,7 +743,7 @@ static int test_update ()
     uint64_t resource6 = 50000;
     int64_t avail, avail1 = 0;
     const char resource_type[] = "core";
-    planner_t *ctx, *ctx2 = NULL;
+    planner_t *ctx = NULL, *ctx2 = NULL;
 
     ctx = planner_new (0, INT64_MAX, resource_total, resource_type);
     // Add some spans