Skip to content

Commit

Permalink
Merge pull request #5824 from grondo/rlist-optimize
Browse files Browse the repository at this point in the history
optimize key librlist functions to improve `flux resource list` performance
  • Loading branch information
mergify[bot] committed Mar 25, 2024
2 parents f607734 + 19ed6cf commit d115427
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
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

0 comments on commit d115427

Please sign in to comment.