From 1483ff0426288d6823cfcd57ec66d6ceaf5bb5dc Mon Sep 17 00:00:00 2001 From: Daniel Milroy Date: Sat, 30 Mar 2024 02:18:13 -0700 Subject: [PATCH 1/3] traverser: add support for dynamic match changes --- resource/traversers/dfu.cpp | 10 ++++++++++ resource/traversers/dfu.hpp | 2 ++ resource/traversers/dfu_impl.cpp | 20 ++++++++++++++++++++ resource/traversers/dfu_impl.hpp | 4 ++++ 4 files changed, 36 insertions(+) diff --git a/resource/traversers/dfu.cpp b/resource/traversers/dfu.cpp index 061b6844b..f4dd18709 100644 --- a/resource/traversers/dfu.cpp +++ b/resource/traversers/dfu.cpp @@ -227,6 +227,16 @@ void dfu_traverser_t::set_match_cb (std::shared_ptr m) detail::dfu_impl_t::set_match_cb (m); } +void dfu_traverser_t::swap_match_cb (const std::string &m) +{ + detail::dfu_impl_t::swap_match_cb (m); +} + +void dfu_traverser_t::restore_match_cb () +{ + detail::dfu_impl_t::restore_match_cb (); +} + void dfu_traverser_t::clear_err_message () { detail::dfu_impl_t::clear_err_message (); diff --git a/resource/traversers/dfu.hpp b/resource/traversers/dfu.hpp index e0fc1ceb8..55a1ab356 100644 --- a/resource/traversers/dfu.hpp +++ b/resource/traversers/dfu.hpp @@ -46,6 +46,8 @@ class dfu_traverser_t : protected detail::dfu_impl_t void set_graph (std::shared_ptr g); void set_graph_db (std::shared_ptr db); void set_match_cb (std::shared_ptr m); + void swap_match_cb (const std::string &m); + void restore_match_cb (); void clear_err_message (); bool is_initialized () const; diff --git a/resource/traversers/dfu_impl.cpp b/resource/traversers/dfu_impl.cpp index e0494613a..e076e38e9 100644 --- a/resource/traversers/dfu_impl.cpp +++ b/resource/traversers/dfu_impl.cpp @@ -15,6 +15,7 @@ extern "C" { } #include "resource/traversers/dfu_impl.hpp" +#include "resource/policies/dfu_match_policy_factory.hpp" using namespace Flux::Jobspec; using namespace Flux::resource_model; @@ -1015,6 +1016,7 @@ dfu_impl_t::dfu_impl_t (const dfu_impl_t &o) m_graph = o.m_graph; m_graph_db = o.m_graph_db; m_match = o.m_match; + m_match_bak = o.m_match_bak; m_err_msg = o.m_err_msg; } @@ -1026,6 +1028,7 @@ dfu_impl_t &dfu_impl_t::operator= (const dfu_impl_t &o) m_graph = o.m_graph; m_graph_db = o.m_graph_db; m_match = o.m_match; + m_match_bak = o.m_match_bak; m_err_msg = o.m_err_msg; return *this; } @@ -1084,6 +1087,23 @@ void dfu_impl_t::set_graph_db (std::shared_ptr db) void dfu_impl_t::set_match_cb (std::shared_ptr m) { m_match = m; + m_match_bak = m; +} + +void dfu_impl_t::swap_match_cb (const std::string &m) +{ + if ( (m_match == nullptr) || (m_match_bak == nullptr)) + return; + + *m_match_bak = *m_match; + m_match = create_match_cb (m); + // Copies the state but not the match type + *m_match = *m_match_bak; +} + +void dfu_impl_t::restore_match_cb () +{ + m_match = m_match_bak; } void dfu_impl_t::clear_err_message () diff --git a/resource/traversers/dfu_impl.hpp b/resource/traversers/dfu_impl.hpp index 7b7b11c48..8ac58d20c 100644 --- a/resource/traversers/dfu_impl.hpp +++ b/resource/traversers/dfu_impl.hpp @@ -134,6 +134,8 @@ class dfu_impl_t { void clear_err_message (); void reset_color (); int reset_exclusive_resource_types (const std::set &x_types); + void swap_match_cb (const std::string &m); + void restore_match_cb (); /*! Exclusive request? Return true if a resource in resources vector * matches resource vertex u and its exclusivity field value is TRUE. @@ -499,6 +501,8 @@ class dfu_impl_t { std::shared_ptr m_graph = nullptr; std::shared_ptr m_graph_db = nullptr; std::shared_ptr m_match = nullptr; + // For swapping matchers + std::shared_ptr m_match_bak = nullptr; expr_eval_api_t m_expr_eval; std::string m_err_msg = ""; }; // the end of class dfu_impl_t From 29fdd933b21166ca81bb365dae18b91141c5aaba Mon Sep 17 00:00:00 2001 From: Daniel Milroy Date: Sat, 30 Mar 2024 02:21:12 -0700 Subject: [PATCH 2/3] traverser: always use first match policy for satisfiability --- resource/traversers/dfu.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/resource/traversers/dfu.cpp b/resource/traversers/dfu.cpp index f4dd18709..526c9feef 100644 --- a/resource/traversers/dfu.cpp +++ b/resource/traversers/dfu.cpp @@ -40,6 +40,11 @@ int dfu_traverser_t::is_satisfiable (Jobspec::Jobspec &jobspec, int saved_errno = errno; const subsystem_t &dom = get_match_cb ()->dom_subsystem (); + // Every satisfiability check should use first match + // for performance. + if (get_match_cb ()->matcher_name () != "first") + swap_match_cb ("first"); + meta.alloc_type = jobmeta_t::alloc_type_t::AT_SATISFIABILITY; planner_multi_t *p = (*get_graph ())[root].idata.subplans.at (dom); meta.at = planner_multi_base_time (p) @@ -54,6 +59,9 @@ int dfu_traverser_t::is_satisfiable (Jobspec::Jobspec &jobspec, m_total_preorder = detail::dfu_impl_t::get_preorder_count (); m_total_postorder = detail::dfu_impl_t::get_postorder_count (); + if (get_match_cb ()->matcher_name () != "first") + restore_match_cb (); + if (!errno) errno = saved_errno; return rc; @@ -84,17 +92,7 @@ int dfu_traverser_t::schedule (Jobspec::Jobspec &jobspec, case match_op_t::MATCH_ALLOCATE_W_SATISFIABILITY: { /* With satisfiability check */ errno = EBUSY; - meta.alloc_type = jobmeta_t::alloc_type_t::AT_SATISFIABILITY; - p = (*get_graph ())[root].idata.subplans.at (dom); - meta.at = planner_multi_base_time (p) - + planner_multi_duration (p) - meta.duration - 1; - detail::dfu_impl_t::count_relevant_types (p, dfv, agg); - if (detail::dfu_impl_t::select (jobspec, root, meta, x) < 0) { - errno = (errno == EBUSY)? ENODEV : errno; - detail::dfu_impl_t::update (); - } - m_total_preorder += detail::dfu_impl_t::get_preorder_count (); - m_total_postorder += detail::dfu_impl_t::get_postorder_count (); + is_satisfiable (jobspec, meta, x, root, dfv); break; } case match_op_t::MATCH_ALLOCATE_ORELSE_RESERVE: { From 4219d35b25ed0e6787f1003c1d130635297edff4 Mon Sep 17 00:00:00 2001 From: Daniel Milroy Date: Sat, 30 Mar 2024 02:22:09 -0700 Subject: [PATCH 3/3] testsuite: update satisfiability check visit counts --- t/t3029-resource-prune.t | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/t/t3029-resource-prune.t b/t/t3029-resource-prune.t index e0a2915d5..25bc06d19 100755 --- a/t/t3029-resource-prune.t +++ b/t/t3029-resource-prune.t @@ -163,14 +163,14 @@ test_expect_success 'prune: use gpu-level pruning' ' cmds07="${cmd_dir}/cmds04.in" test_expect_success 'prune: pruning not affected by satisfiability (p=high)' ' cat <<-EOF >cmp07 && - INFO: PREORDER VISIT COUNT=100 - INFO: POSTORDER VISIT COUNT=80 - INFO: PREORDER VISIT COUNT=100 - INFO: POSTORDER VISIT COUNT=80 - INFO: PREORDER VISIT COUNT=100 - INFO: POSTORDER VISIT COUNT=80 - INFO: PREORDER VISIT COUNT=100 - INFO: POSTORDER VISIT COUNT=80 + INFO: PREORDER VISIT COUNT=27 + INFO: POSTORDER VISIT COUNT=22 + INFO: PREORDER VISIT COUNT=27 + INFO: POSTORDER VISIT COUNT=22 + INFO: PREORDER VISIT COUNT=27 + INFO: POSTORDER VISIT COUNT=22 + INFO: PREORDER VISIT COUNT=27 + INFO: POSTORDER VISIT COUNT=22 EOF sed "s~@TEST_SRCDIR@~${SHARNESS_TEST_SRCDIR}~g" ${cmds07} > cmds07 && ${query} -L ${grugs} -S CA -P high -t 007.R.out -e < cmds07 > out07 &&