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

Fixed memory leak in LocationMap::remove #251

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ImmemorConsultrixContrarie
Copy link
Contributor

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie commented Mar 25, 2021

Fixes #245.

The initial solution has obvious worst case: inserting a single entity and then removing it, which would allocate the block in the heap and then deallocate the same block.

What could be done about it:

  1. Do nothing. Could be a good option, actually, because inserting a single entity is awfully ineffective and creates a unique block for any single insert anyway.
  2. Use [T; N] instead of Box<[T; N]>. This would make hashmap storage slower to reallocate, but totally avoids single entity allocation problem and lessens indirection.
  3. Make single entity a special case:
enum OneTooMany<T> { One(T), Many(Box<[T; N]>) }

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie changed the title Fixed memory leak in World::remove Fixed memory leak in LocationMap::remove Mar 26, 2021
@TomGillen
Copy link
Collaborator

If we keep a count next to each array, we could maintain that and remove the entry when the count becomes 0, rather than have to scan through the array to check for all nones on every remove.

@ImmemorConsultrixContrarie
Copy link
Contributor Author

If we keep a count next to each array, we could maintain that and remove the entry when the count becomes 0, rather than have to scan through the array to check for all nones on every remove.

That's true and I had a thought about it, but that required too much code to write at the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory isn't freed after entity removal
2 participants