Merge pull request #1061 from milroy/comparison-update

Planner comparison and update

Author: mergify[bot]

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 )

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()

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

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++
 

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);
+}
+
+
 
 /*******************************************************************************
  *                                                                             *

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>

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<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;
@@ -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);

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;

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
+ */

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 */