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.

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

optimize key librlist functions to improve flux resource list performance #5824

Merged
merged 3 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/common/libidset/idset.c
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ bool idset_equal (const struct idset *idset1,
const struct idset *idset2)
{
unsigned int id;
bool count_checked = false;

if (!idset1 || !idset2)
return false;
Expand All @@ -421,6 +422,7 @@ bool idset_equal (const struct idset *idset1,
&& !(idset2->flags & IDSET_FLAG_COUNT_LAZY)) {
if (idset_count (idset1) != idset_count (idset2))
return false;
count_checked = true;
}

id = vebsucc (idset1->T, 0);
Expand All @@ -429,6 +431,13 @@ bool idset_equal (const struct idset *idset1,
return false; // id in idset1 not set in idset2
id = vebsucc (idset1->T, id + 1);
}

/* No need to iterate idset2 if counts were equal and all ids in idset1
* were found in idset2.
*/
if (count_checked)
return true;

id = vebsucc (idset2->T, 0);
while (id < idset2->T.M) {
if (vebsucc (idset1->T, id) != id)
Expand Down
28 changes: 26 additions & 2 deletions src/common/librlist/rnode.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
static struct idset *util_idset_add_check (const struct idset *a,
const struct idset *b)
{
if (idset_count (a) == 0)
return idset_copy (b);
if (idset_has_intersection (a, b)) {
errno = EEXIST;
return NULL;
Expand Down Expand Up @@ -81,6 +83,20 @@ static struct rnode_child *rnode_child_copy (const struct rnode_child *c)
return rnode_child_idset (c->name, c->ids, c->avail);
}

static int rnode_child_clear (struct rnode_child *c)
{
/* clearing idsets manually is expensive. It is cheaper to destroy
* and recraete an empty idset. Update this code when that is no longer
* the case.
*/
idset_destroy (c->avail);
idset_destroy (c->ids);
if (!(c->avail = idset_create (0, IDSET_FLAG_AUTOGROW))
|| !(c->ids = idset_create (0, IDSET_FLAG_AUTOGROW)))
return -1;
return 0;
}

static void rn_child_free (void **x)
{
if (x) {
Expand Down Expand Up @@ -463,8 +479,16 @@ struct rnode *rnode_diff (const struct rnode *a, const struct rnode *b)
while (c) {
struct rnode_child *nc = zhashx_lookup (n->children, c->name);
if (nc) {
if (idset_subtract (nc->ids, c->ids) < 0
|| idset_subtract (nc->avail, c->avail) < 0)
if (idset_equal (nc->ids, c->ids)) {
/* Optimization: if nc->ids == c->ids, then just replace
* nc with empty idsets. This will be much faster than
* idset_subtract().
*/
if (rnode_child_clear (nc) < 0)
goto err;
}
else if (idset_subtract (nc->ids, c->ids) < 0
|| idset_subtract (nc->avail, c->avail) < 0)
goto err;

/* For non-core resources, remove empty sets:
Expand Down