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

Ensure BufferBlocks are completed and empty in RowShufflingTransformer. #4479

Merged
merged 2 commits into from
Nov 22, 2019

Conversation

eerhardt
Copy link
Member

If BufferBlock doesn't get completed and drained of its items, it will have non-completed Tasks. When debugging in VS, this will appear to be a memory leak because VS adds all running Tasks to a static Dictionary, and then removes them when the Task is complete. If the Task doesn't get completed, it won't be removed from the Dictionary - thus it looks like a leak.

Note there is no leak when the VS Debugger isn't attached because the non-completed Tasks don't get added to the static Dictionary.

Fix #4399

Copy link
Member

@antoniovs1029 antoniovs1029 left a comment

Choose a reason for hiding this comment

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

I ran a couple of use cases using the changes in this PR, and it does solve the problem of the memory leak while debugging.

If BufferBlock doesn't get completed and drained of its items, it will have non-completed Tasks. When debugging in VS, this will appear to be a memory leak because VS adds all running Tasks to a static Dictionary, and then removes them when the Task is complete. If the Task doesn't get completed, it won't be removed from the Dictionary - thus it looks like a leak.

Note there is no leak when the VS Debugger isn't attached because the non-completed Tasks don't get added to the static Dictionary.

Fix dotnet#4399
@codemzs codemzs closed this Nov 22, 2019
@codemzs codemzs reopened this Nov 22, 2019
@eerhardt eerhardt merged commit 1fa6cb5 into dotnet:master Nov 22, 2019
@eerhardt eerhardt deleted the Fix4399 branch November 22, 2019 20:18
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak in text classification pipeline
4 participants