Skip to content

Commit

Permalink
lib: Avoid O(N^2) behavior on cleanup
Browse files Browse the repository at this point in the history
This library was written in a very general way with an API
`lcfs_node_remove_child` that required removing an arbitrary
member of the children array.  I found a bug in that code
previously in that it should have been using `memmove`, not
`memcpy`.

However, I think there's no need for us to support arbitrary
tree computations.

Today at process exit we walk over the in-memory filesystem
and we end up with an O(N^2) behavior because we keep removing
the first child.

Instead, because we *only* need to care about removing all
children at once, just iterate over them and then free them
as we go.

Note: I only noticed this code because of the `memmove` fix, not
because I saw some performance problem in practice.  It turns
out computers are fast...

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cgwalters committed Jun 21, 2023
1 parent 447b119 commit 46700ba
Showing 1 changed file with 8 additions and 22 deletions.
30 changes: 8 additions & 22 deletions libcomposefs/lcfs-writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -754,25 +754,6 @@ void lcfs_node_make_hardlink(struct lcfs_node_s *node, struct lcfs_node_s *targe
target->inode.st_nlink++;
}

static void lcfs_node_remove_child_node(struct lcfs_node_s *parent, int offset,
struct lcfs_node_s *child)
{
assert(child->parent == parent);
assert(parent->children[offset] == child);

memmove(&parent->children[offset], &parent->children[offset + 1],
sizeof(struct lcfs_node_s *) *
(parent->children_size - (offset + 1)));
parent->children_size -= 1;

/* Unlink correctly as it may live on outside the tree and be reinserted */
free(child->name);
child->name = NULL;
child->parent = NULL;

lcfs_node_unref(child);
}

int lcfs_node_add_child(struct lcfs_node_s *parent, struct lcfs_node_s *child,
const char *name)
{
Expand Down Expand Up @@ -867,11 +848,16 @@ void lcfs_node_unref(struct lcfs_node_s *node)

static void lcfs_node_remove_all_children(struct lcfs_node_s *node)
{
while (node->children_size > 0) {
struct lcfs_node_s *child = lcfs_node_ref(node->children[0]);
lcfs_node_remove_child_node(node, 0, child);
for (size_t i = 0; i < node->children_size; i++) {
struct lcfs_node_s *child = node->children[i];
assert(child->parent == node);
/* Unlink correctly as it may live on outside the tree and be reinserted */
free(child->name);
child->name = NULL;
child->parent = NULL;
lcfs_node_destroy(child);
}
node->children_size = 0;
}

/* Unlink all children (recursively) and then unref. Useful to handle refcount loops like dot and dotdot. */
Expand Down

0 comments on commit 46700ba

Please sign in to comment.