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

Fix memory leak in builders #193

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Fix memory leak in builders #193

merged 1 commit into from
Sep 4, 2024

Conversation

zajac
Copy link
Contributor

@zajac zajac commented Sep 4, 2024

Builders hold on to the original collection, causing memory leaks in our use case.

In order to make persistent collections composable (to put one as an element of another) and preserve the efficiency of temporary mutability we have to compose builders, instead of persistent collections. Since we hold on to this builders for a long time, the reference to the old collection in all builders causes memory leaks.

The proposed fix is to release the reference, once the collection is changed and there is no chance to return it from build fn.

Copy link
Contributor

@qurbonzoda qurbonzoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Releasing the previously built collection at the point when the builder gets updated makes sense to me. However, this adds some complexity to the implementation.

kotlin-js-store/yarn.lock Outdated Show resolved Hide resolved
@zajac zajac force-pushed the master branch 2 times, most recently from 16b3f1d to 4c06b10 Compare September 4, 2024 15:53
@qurbonzoda qurbonzoda merged commit fe7b163 into Kotlin:master Sep 4, 2024
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.

2 participants