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

Implement GC for RHT Node #796

Closed
raararaara opened this issue Feb 16, 2024 · 2 comments
Closed

Implement GC for RHT Node #796

raararaara opened this issue Feb 16, 2024 · 2 comments
Labels
enhancement 🌟 New feature or request good first issue 🐤 Good for newcomers sdk ⚒️

Comments

@raararaara
Copy link
Contributor

raararaara commented Feb 16, 2024

What would you like to be added:

Add a feature to RHT that allows the removal of attributes from elements

Currently, when performing Tree.RemoveStyle and calling RHT.Remove, the actual removal from the map is not performed. This is to avoid potential convergence issues in concurrent editing scenarios. Instead, the RHT Node is marked as isRemoved.

newNode := newRHTNode(k, node.val, executedAt, true)
rht.nodeMapByKey[k] = newNode

However, this approach of just marking the nodes as isRemoved can lead to memory issues if the number of marked RHT Nodes keeps increasing. Therefore, it is necessary to implement a garbage collector that handles tombstones to remove the marked nodes from memory.

Why is this needed:

To address potential memory issues caused by the increasing number of marked RHT Nodes and improve the efficiency of the RHT implementation.

@raararaara
Copy link
Contributor Author

raararaara commented Mar 26, 2024

Since Go automatically handles the garbage collection for objects that are not being referenced, there's no need for specific garbage collection implementation for rhtNode. Hence, I'm closing this issue.

Additionally, it seems that garbage collection for ElementRHT, which is already implemented, may not be necessary. I'll address the garbage collection removal for ElementRHT in a new issue.

The case where the reference count becomes 0 is when the value corresponding to the existing key is overwritten in the process of RHT.set. Therefore, there is no need for gc logic in RHT.set.

In the case of RHT.Remove, however, it is still a required specification in the concurrent editing scenario, so I reopen the issue.

@hackerwins
Copy link
Member

This was fixed by #864.

@github-project-automation github-project-automation bot moved this from Todo to Done in Yorkie Project Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🌟 New feature or request good first issue 🐤 Good for newcomers sdk ⚒️
Projects
No open projects
Status: Done
Development

No branches or pull requests

2 participants