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

Improve memory consumption by cleaning up garbage references to pending promise without canceller #34

Merged
merged 1 commit into from
Jun 13, 2018

Conversation

clue
Copy link
Member

@clue clue commented Jun 12, 2018

The previous changes in #33 improved memory consumption for settled promises somewhat. Similarly, this PR addresses memory consumption for pending promises that are no longer referenced.

Calling timeout() on a promise which has no canceller function does successfully reject the promise. However, due to internal references, it keeps a cyclic reference to the input promise and as such shows some unexpected memory consumption and memory would not immediately be freed as expected. Let's not call this a "memory leak", because memory was eventually freed, but this clearly caused some unexpected and significant memory growth.

I'm marking this PR as WIP because this includes a test for reactphp/promise#124 which is yet to be released as part of react/promise v2.7.0. Once this release is out, I'll update the version reference and this should be ready to be shipped.

Builds on top of #33

@clue clue changed the title [WIP] Improve memory consumption by cleaning up garbage references to pending promise without canceller Improve memory consumption by cleaning up garbage references to pending promise without canceller Jun 13, 2018
@clue
Copy link
Member Author

clue commented Jun 13, 2018

Updated and removed WIP marker now that https://github.com/reactphp/promise/releases/tag/v2.7.0 has been released, this is now ready :shipit:

@clue clue requested review from jsor and WyriHaximus June 13, 2018 16:12
@WyriHaximus WyriHaximus merged commit 170fb93 into reactphp:master Jun 13, 2018
@clue clue deleted the garbage branch June 13, 2018 16:44
@clue clue mentioned this pull request Feb 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants