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/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++/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 diff --git a/resource/planner/c++/planner.cpp b/resource/planner/c++/planner.cpp index 87e2166d1..4d6963daa 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 * @@ -118,7 +149,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; @@ -132,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 @@ -193,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 (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; @@ -428,25 +488,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 ()) @@ -460,23 +501,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 (!scheduled_points_equal (*(this_it.second->start_p), - *(other->second->start_p))) - return false; - if (!scheduled_points_equal (*(this_it.second->last_p), - *(other->second->last_p))) + // Compare span_t + if (*(this_it.second) != *(other->second)) return false; } } @@ -497,8 +523,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 +543,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..6d3fceb94 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 */ @@ -42,11 +45,13 @@ 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 (); 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; @@ -108,8 +113,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; 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 */ diff --git a/resource/planner/c++/planner_multi.cpp b/resource/planner/c++/planner_multi.cpp index ff7c1966b..f3ad657b2 100644 --- a/resource/planner/c++/planner_multi.cpp +++ b/resource/planner/c++/planner_multi.cpp @@ -35,68 +35,55 @@ 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[type] = 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 = o.m_span_lookup; m_span_lookup_iter = o.m_span_lookup_iter; m_span_counter = o.m_span_counter; } @@ -106,40 +93,34 @@ 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 = o.m_span_lookup; m_span_lookup_iter = o.m_span_lookup_iter; m_span_counter = o.m_span_counter; @@ -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,47 +140,41 @@ 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 (); + auto &by_type = m_types_totals_planners.get (); + 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; } return true; } +bool planner_multi::operator!= (const planner_multi &o) const +{ + return !operator == (o); +} + 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 () @@ -209,49 +182,108 @@ planner_multi::~planner_multi () erase (); } -planner_t *planner_multi::get_planners_at (size_t i) +void planner_multi::add_planner (int64_t base_time, uint64_t duration, + const uint64_t resource_total, + const char *resource_type, size_t i) { - return m_planners.at (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 &rtypes) +{ + auto &by_res = m_types_totals_planners.get (); + 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; + } } -std::vector &planner_multi::get_planners () +planner_t *planner_multi::get_planner_at (size_t i) const { - return m_planners; + return m_types_totals_planners.at (i).planner; } -size_t planner_multi::get_planners_size () +planner_t *planner_multi::get_planner_at (const char *type) const { - return m_planners.size (); + auto &by_res = m_types_totals_planners.get (); + return by_res.find (std::string (type))->planner; } -void planner_multi::resource_totals_push_back (const uint64_t resource_total) +void planner_multi::update_planner_index (const char *type, size_t i) { - m_resource_totals.push_back (resource_total); + std::string rtype = std::string (type); + auto by_res = m_types_totals_planners.get ().find (rtype); + auto new_idx = m_types_totals_planners.begin () + i; + auto curr_idx = m_types_totals_planners.get ().iterator_to (*by_res); + // noop if new_idx == curr_idx + m_types_totals_planners.relocate (new_idx, curr_idx); } -uint64_t planner_multi::get_resource_totals_at (size_t i) +int planner_multi::update_planner_total (uint64_t total, size_t i) { - return m_resource_totals.at (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 (); + auto result = by_res.find (std::string (type)); + if (result == by_res.end ()) + return false; + else + return true; } -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 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 (); + 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 d27122563..a2bb66069 100644 --- a/resource/planner/c++/planner_multi.hpp +++ b/resource/planner/c++/planner_multi.hpp @@ -12,13 +12,44 @@ #define PLANNER_MULTI_HPP #include "planner.hpp" +#include +#include +#include +#include +#include +#include struct request_multi { int64_t on_or_after = 0; uint64_t duration = 0; - std::vector counts; + std::unordered_map 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 // index nametag + >, + hashed_unique< // unordered_set-like; faster than ordered_unique in testing + tag, // index nametag + member // index's key + > + > +> multi_container; + class planner_multi { public: planner_multi (); @@ -28,19 +59,20 @@ 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 (); // Public getters and setters - planner_t *get_planners_at (size_t i); - std::vector &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 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; + 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; + const char *get_resource_type_at (size_t i) const; struct request_multi &get_iter (); // Span lookup functions std::map> &get_span_lookup (); @@ -53,14 +85,15 @@ class planner_multi { uint64_t get_span_counter (); 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 m_resource_types; - std::vector m_resource_totals; - std::vector m_planners; + 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 &rtypes); private: + multi_container m_types_totals_planners; struct request_multi m_iter; std::map> m_span_lookup; std::map>::iterator m_span_lookup_iter; 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 { 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::RBTreeplan) == *(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 */ diff --git a/resource/planner/c/planner_multi.h b/resource/planner/c/planner_multi.h index 94a541df2..8b2d1de40 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, @@ -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 95886ce23..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 #include #include +#include #include "planner_multi.h" #include "resource/planner/c++/planner_multi.hpp" @@ -32,7 +33,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 ( @@ -118,7 +120,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 (0))); } extern "C" int64_t planner_multi_duration (planner_multi_t *ctx) @@ -127,7 +129,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 (0))); } extern "C" size_t planner_multi_resources_len (planner_multi_t *ctx) @@ -139,22 +141,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 +160,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 +169,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 +191,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 +215,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 +238,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 (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 (0)))) != -1); done: return t; @@ -272,20 +263,22 @@ 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; 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 (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, + 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); @@ -301,7 +294,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 +308,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 +327,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 +342,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 +380,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 +406,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); @@ -473,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 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 (0))); + duration = planner_duration ( + ctx->plan_multi->get_planner_at (static_cast (0))); + if (duration < 0) { + errno = EINVAL; + goto done; + } + + for (i = 0; i < len; ++i) { + if (resource_totals[i] > + static_cast (std::numeric_limits::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 (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 */ diff --git a/resource/planner/test/planner_test01.cpp b/resource/planner/test/planner_test01.cpp index 97793d715..c7077c485 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); @@ -581,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; @@ -639,6 +643,8 @@ static int test_more_add_remove () bo = (bo || span6 == -1); ok (!bo, "more add-remove-add test works"); + + planner_destroy (&ctx); return 0; } @@ -655,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); @@ -723,9 +729,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 = NULL, *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 +808,8 @@ int main (int argc, char *argv[]) test_constructors_and_overload (); + test_update (); + done_testing (); return EXIT_SUCCESS; diff --git a/resource/planner/test/planner_test02.cpp b/resource/planner/test/planner_test02.cpp index dd4db132d..76b42a420 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; } @@ -551,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); @@ -615,10 +621,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 +717,8 @@ int main (int argc, char *argv[]) test_constructors_and_overload (); + test_multi_update (); + done_testing (); return EXIT_SUCCESS; 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); 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