Bring back value serialization #19
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit cdb1aad.
|
I will try to find some time over the next three days to look at / work on this. Thanks! |
kostrzewa
reviewed
Oct 21, 2019
|
|
||
| result <- func(param_row, value_loaded) | ||
|
|
||
| rm(value_loaded) |
There was a problem hiding this comment.
This is likely not sufficient to keep memory consumption low...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@kostrzewa has PV objects of 40 GB, which become hard to handle to memory. We discussed to bring back the value serialization and convince the garbage collector to make it work.
In this pull request we can discuss the finer details for this work in progress. So far I have just revered the commit which removed the logic and fixed the merge conflicts. The code will not run.
One of the things that annoyed me was that the
pv_callwould have to know the name of the variable that one stores stuff into. This time I'd favor a new argumentfilenamewhich when given will be the place where stuff is stored, withoutput/values/FILENAME/value-%d.Rdata. Perhaps we should call it “dirname”.pv_callwill delete the contents of the directory and then serialize the values into there.I think last time I serialized each named element of each value into a separate file, like
value-%d-%s.Rdata. Instead we could also just store each element of the numbered outer list into a single file. This would result in less files if there are multiple named values. In the previous version it would check the size of the each named value in the first list element and deduce the amount of storage needed. I think that this logic should just be removed. Either store each named list as a single file or do nothing at all.Would that suit your needs?