From 8efe8e63d3506d51a37692afa0594edbf12c945f Mon Sep 17 00:00:00 2001 From: Bill Currie Date: Sun, 5 Mar 2023 21:29:26 +0900 Subject: [PATCH] [ecs] Plug a bunch of memory leaks The hierarchy leak was particularly troublesome to fix, but now the hierarchies get updated (and freed) automatically just by removing the hierarchy reference component from the entity. I suspect there will be issues with entities that are on multiple hierarchies, but I'll sort that out later. --- include/QF/ecs.h | 1 + include/QF/ecs/hierarchy.h | 2 ++ libs/console/server.c | 1 + libs/ecs/ecs.c | 20 +++++++++++++++++--- libs/ecs/entity.c | 4 ++++ libs/ecs/hierarchy.c | 34 ++++++++++++++++++++++++++++++++-- libs/ecs/test/test-hierarchy.c | 1 + libs/scene/scene.c | 1 + libs/ui/passage.c | 1 + libs/ui/test/test-flow-size.c | 1 + libs/ui/test/test-flow.c | 1 + libs/ui/view.c | 1 + 12 files changed, 63 insertions(+), 5 deletions(-) diff --git a/include/QF/ecs.h b/include/QF/ecs.h index c5402daaa..e321f663e 100644 --- a/include/QF/ecs.h +++ b/include/QF/ecs.h @@ -87,6 +87,7 @@ typedef struct ecs_registry_s { uint32_t max_entities; componentset_t components; PR_RESMAP (hierarchy_t) hierarchies;//FIXME find a better way + int locked; } ecs_registry_t; /** Tie an ECS system to a registry. diff --git a/include/QF/ecs/hierarchy.h b/include/QF/ecs/hierarchy.h index 6036a8f91..88e9c5665 100644 --- a/include/QF/ecs/hierarchy.h +++ b/include/QF/ecs/hierarchy.h @@ -77,6 +77,8 @@ void Hierarchy_RemoveHierarchy (hierarchy_t *hierarchy, uint32_t index, hierref_t Hierarchy_SetParent (hierarchy_t *dst, uint32_t dstParent, hierarchy_t *src, uint32_t srcIndex); +void Hierref_DestroyComponent (void *href); + ///@} #endif//__QF_ecs_hierarchy_h diff --git a/libs/console/server.c b/libs/console/server.c index fc7e90d12..8a464dda7 100644 --- a/libs/console/server.c +++ b/libs/console/server.c @@ -224,6 +224,7 @@ static const component_t server_components[server_comp_count] = { [server_href] = { .size = sizeof (hierref_t), .name = "href", + .destroy = Hierref_DestroyComponent, }, [server_view] = { .size = sizeof (sv_view_t), diff --git a/libs/ecs/ecs.c b/libs/ecs/ecs.c index a53d83ae0..0a4afaa04 100644 --- a/libs/ecs/ecs.c +++ b/libs/ecs/ecs.c @@ -46,17 +46,26 @@ ECS_NewRegistry (void) VISIBLE void ECS_DelRegistry (ecs_registry_t *registry) { + registry->locked = 1; + for (uint32_t i = 0; i < registry->components.size; i++) { + __auto_type comp = ®istry->components.a[i]; + __auto_type pool = ®istry->comp_pools[i]; + Component_DestroyElements (comp, pool->data, 0, pool->count); + } free (registry->entities); for (uint32_t i = 0; i < registry->components.size; i++) { free (registry->comp_pools[i].sparse); free (registry->comp_pools[i].dense); free (registry->comp_pools[i].data); + free (registry->subpools[i].sorted); free (registry->subpools[i].ranges); free (registry->subpools[i].rangeids); } + DARRAY_CLEAR (®istry->components); free (registry->subpools); free (registry->comp_pools); + PR_RESDELMAP (registry->hierarchies); free (registry); } @@ -193,15 +202,20 @@ ECS_NewEntity (ecs_registry_t *registry) VISIBLE void ECS_DelEntity (ecs_registry_t *registry, uint32_t ent) { + if (registry->locked) { + // the registry is being deleted and mass entity and component + // deletions are going on + return; + } uint32_t next = registry->next | Ent_NextGen (Ent_Generation (ent)); uint32_t id = Ent_Index (ent); - registry->entities[id] = next; - registry->next = id; - registry->available++; for (uint32_t i = 0; i < registry->components.size; i++) { Ent_RemoveComponent (ent, i, registry); } + registry->entities[id] = next; + registry->next = id; + registry->available++; } VISIBLE void diff --git a/libs/ecs/entity.c b/libs/ecs/entity.c index a9c54a58f..5fb7a395b 100644 --- a/libs/ecs/entity.c +++ b/libs/ecs/entity.c @@ -117,6 +117,10 @@ Ent_RemoveComponent (uint32_t ent, uint32_t comp, ecs_registry_t *registry) component_t *c = ®istry->components.a[comp]; if (ind < pool->count && pool->dense[ind] == ent) { uint32_t last = pool->count - 1; + // invalidate the entity for this component to prevent the component + // being double-removed due to deletion of the component resulting + // in the entity being deleted (happens with hierarchies) + pool->dense[ind] = -1; Component_DestroyElements (c, pool->data, ind, 1); uint32_t range_count = subpool->num_ranges - subpool->available; // if ind >= the last range, then it is outside the subpools diff --git a/libs/ecs/hierarchy.c b/libs/ecs/hierarchy.c index 19b8079fc..58caca7ff 100644 --- a/libs/ecs/hierarchy.c +++ b/libs/ecs/hierarchy.c @@ -58,6 +58,21 @@ hierarchy_UpdateTransformIndices (hierarchy_t *hierarchy, uint32_t start, } } +static void +hierarchy_InvalidateReferences (hierarchy_t *hierarchy, uint32_t start, + uint32_t count) +{ + ecs_registry_t *reg = hierarchy->reg; + uint32_t href = hierarchy->href_comp; + for (size_t i = start; count-- > 0; i++) { + if (ECS_EntValid (hierarchy->ent[i], reg)) { + hierref_t *ref = Ent_GetComponent (hierarchy->ent[i], href, reg); + ref->hierarchy = 0; + ref->index = -1; + } + } +} + static void hierarchy_UpdateChildIndices (hierarchy_t *hierarchy, uint32_t start, int offset) @@ -165,7 +180,7 @@ hierarchy_move (hierarchy_t *dst, const hierarchy_t *src, src->ent, srcIndex, count); // Actually move (as in C++ move semantics) source hierarchy object // references so that their indices do not get updated when the objects - // are removed from the source hierarcy + // are removed from the source hierarchy memset (&src->ent[srcIndex], nullent, count * sizeof(dst->ent[0])); for (uint32_t i = 0; i < count; i++) { @@ -325,6 +340,7 @@ hierarchy_remove_children (hierarchy_t *hierarchy, uint32_t index, hierarchy_remove_children (hierarchy, childIndex + i, delEntities); } if (delEntities) { + hierarchy_InvalidateReferences (hierarchy, childIndex, childCount); for (uint32_t i = 0; i < childCount; i++) { ECS_DelEntity (hierarchy->reg, hierarchy->ent[childIndex + i]); } @@ -349,6 +365,7 @@ Hierarchy_RemoveHierarchy (hierarchy_t *hierarchy, uint32_t index, hierarchy_remove_children (hierarchy, index, delEntities); if (delEntities) { + hierarchy_InvalidateReferences (hierarchy, index, 1); ECS_DelEntity (hierarchy->reg, hierarchy->ent[index]); } hierarchy_close (hierarchy, index, 1); @@ -386,6 +403,7 @@ Hierarchy_New (ecs_registry_t *reg, uint32_t href_comp, void Hierarchy_Delete (hierarchy_t *hierarchy) { + hierarchy_InvalidateReferences (hierarchy, 0, hierarchy->num_objects); free (hierarchy->ent); free (hierarchy->childCount); free (hierarchy->childIndex); @@ -440,7 +458,7 @@ Hierarchy_SetParent (hierarchy_t *dst, uint32_t dstParent, hierref_t r = {}; if (dst && dstParent != nullent) { if (dst->type != src->type) { - Sys_Error ("Can't set parent in hierarcy of different type"); + Sys_Error ("Can't set parent in hierarchy of different type"); } } else { if (!srcRoot) { @@ -458,3 +476,15 @@ Hierarchy_SetParent (hierarchy_t *dst, uint32_t dstParent, } return r; } + +void +Hierref_DestroyComponent (void *href) +{ + hierref_t ref = *(hierref_t *) href; + if (ref.hierarchy) { + Hierarchy_RemoveHierarchy (ref.hierarchy, ref.index, 1); + if (!ref.hierarchy->num_objects) { + Hierarchy_Delete (ref.hierarchy); + } + } +} diff --git a/libs/ecs/test/test-hierarchy.c b/libs/ecs/test/test-hierarchy.c index ad3aa28f3..3d9be814a 100644 --- a/libs/ecs/test/test-hierarchy.c +++ b/libs/ecs/test/test-hierarchy.c @@ -21,6 +21,7 @@ static const component_t test_components[] = { .size = sizeof (hierref_t), .create = 0,//create_href, .name = "href", + .destroy = Hierref_DestroyComponent, }, [test_name] = { .size = sizeof (const char *), diff --git a/libs/scene/scene.c b/libs/scene/scene.c index 6704e7e48..d7cfa519f 100644 --- a/libs/scene/scene.c +++ b/libs/scene/scene.c @@ -99,6 +99,7 @@ static const component_t scene_components[scene_comp_count] = { .size = sizeof (hierref_t), .create = 0,//create_href, .name = "href", + .destroy = Hierref_DestroyComponent, }, [scene_animation] = { .size = sizeof (animation_t), diff --git a/libs/ui/passage.c b/libs/ui/passage.c index 089823267..d3c374d80 100644 --- a/libs/ui/passage.c +++ b/libs/ui/passage.c @@ -48,6 +48,7 @@ const component_t passage_components[passage_comp_count] = { [passage_href] = { .size = sizeof (hierref_t), .name = "passage href", + .destroy = Hierref_DestroyComponent, }, }; diff --git a/libs/ui/test/test-flow-size.c b/libs/ui/test/test-flow-size.c index 40c133aa8..2c6bbf092 100644 --- a/libs/ui/test/test-flow-size.c +++ b/libs/ui/test/test-flow-size.c @@ -18,6 +18,7 @@ static const component_t test_components[] = { .size = sizeof (hierref_t), .create = 0,//create_href, .name = "href", + .destroy = Hierref_DestroyComponent, }, }; diff --git a/libs/ui/test/test-flow.c b/libs/ui/test/test-flow.c index e082447d8..bbcfdeba2 100644 --- a/libs/ui/test/test-flow.c +++ b/libs/ui/test/test-flow.c @@ -18,6 +18,7 @@ static const component_t test_components[] = { .size = sizeof (hierref_t), .create = 0,//create_href, .name = "href", + .destroy = Hierref_DestroyComponent, }, }; diff --git a/libs/ui/view.c b/libs/ui/view.c index 70b8d8326..337cb53f6 100644 --- a/libs/ui/view.c +++ b/libs/ui/view.c @@ -94,6 +94,7 @@ const component_t view_components[view_comp_count] = { [view_href] = { .size = sizeof (hierref_t), .name = "view href", + .destroy = Hierref_DestroyComponent, }, };