-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
clear out memory after reads from the dagreader #4525
Conversation
Maybe a test could add a partial dag, make requests into ranges of it, and then verify we only hold roughly as much memory as we think is appropriate. |
87f77b4
to
b8b54a1
Compare
License: MIT Signed-off-by: Jeromy <[email protected]>
b8b54a1
to
092df58
Compare
Added a test, another round of review please |
unixfs/io/dagreader_test.go
Outdated
} | ||
|
||
pbdr := reader.(*pbDagReader) | ||
if pbdr.promises[0] != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we checked the correct number of things to be loaded instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit, LGTM.
License: MIT Signed-off-by: Jeromy <[email protected]>
c7a3c2b
to
dea8c4d
Compare
I would like to figure out how to add a test or two for this. I feel like testing the internals of the dagreader object is a bit pointless, and its pretty difficult to test if something has been garbage collected.
License: MIT
Signed-off-by: Jeromy [email protected]