Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add support for dynamic match policy changes and always use first match for feasibility/satisfiability #1160

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Add support for dynamic match policy changes and always use first match for feasibility/satisfiability #1160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1483ff0
traverser: add support for dynamic match changes
milroy Mar 30, 2024
29fdd93
traverser: always use first match policy for satisfiability
milroy Mar 30, 2024
4219d35
testsuite: update satisfiability check visit counts
milroy Mar 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions resource/traversers/dfu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -227,6 +225,16 @@ void dfu_traverser_t::set_match_cb (std::shared_ptr<dfu_match_cb_t> 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 ();
Expand Down
2 changes: 2 additions & 0 deletions resource/traversers/dfu.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class dfu_traverser_t : protected detail::dfu_impl_t
void set_graph (std::shared_ptr<f_resource_graph_t> g);
void set_graph_db (std::shared_ptr<resource_graph_db_t> db);
void set_match_cb (std::shared_ptr<dfu_match_cb_t> m);
void swap_match_cb (const std::string &m);
void restore_match_cb ();
void clear_err_message ();

bool is_initialized () const;
Expand Down
20 changes: 20 additions & 0 deletions resource/traversers/dfu_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
}

#include "resource/traversers/dfu_impl.hpp"
#include "resource/policies/dfu_match_policy_factory.hpp"

using namespace Flux::Jobspec;
using namespace Flux::resource_model;
Expand Down Expand Up @@ -1015,6 +1016,7 @@
m_graph = o.m_graph;
m_graph_db = o.m_graph_db;
m_match = o.m_match;
m_match_bak = o.m_match_bak;

Check warning on line 1019 in resource/traversers/dfu_impl.cpp

View check run for this annotation

Codecov / codecov/patch

resource/traversers/dfu_impl.cpp#L1019 

Added line #L1019 was not covered by tests
m_err_msg = o.m_err_msg;
}

Expand All @@ -1026,6 +1028,7 @@
m_graph = o.m_graph;
m_graph_db = o.m_graph_db;
m_match = o.m_match;
m_match_bak = o.m_match_bak;

Check warning on line 1031 in resource/traversers/dfu_impl.cpp

View check run for this annotation

Codecov / codecov/patch

resource/traversers/dfu_impl.cpp#L1031 

Added line #L1031 was not covered by tests
m_err_msg = o.m_err_msg;
return *this;
}
Expand Down Expand Up @@ -1084,6 +1087,23 @@
void dfu_impl_t::set_match_cb (std::shared_ptr<dfu_match_cb_t> 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 ()
Expand Down
4 changes: 4 additions & 0 deletions resource/traversers/dfu_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ class dfu_impl_t {
void clear_err_message ();
void reset_color ();
int reset_exclusive_resource_types (const std::set<std::string> &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.
Expand Down Expand Up @@ -499,6 +501,8 @@ class dfu_impl_t {
std::shared_ptr<f_resource_graph_t> m_graph = nullptr;
std::shared_ptr<resource_graph_db_t> m_graph_db = nullptr;
std::shared_ptr<dfu_match_cb_t> m_match = nullptr;
// For swapping matchers
std::shared_ptr<dfu_match_cb_t> m_match_bak = nullptr;
expr_eval_api_t m_expr_eval;
std::string m_err_msg = "";
}; // the end of class dfu_impl_t
Expand Down
16 changes: 8 additions & 8 deletions t/t3029-resource-prune.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
Loading