From 46700ba58724a842019f0e84cc7d216ec2fdf5c5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 21 Jun 2023 14:33:25 -0400 Subject: [PATCH] lib: Avoid O(N^2) behavior on cleanup 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 --- libcomposefs/lcfs-writer.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/libcomposefs/lcfs-writer.c b/libcomposefs/lcfs-writer.c index 05fed1a6..6ebb868f 100644 --- a/libcomposefs/lcfs-writer.c +++ b/libcomposefs/lcfs-writer.c @@ -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) { @@ -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. */