Reduce memory usage for large directories/files #3773
Merged
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
MichaelEischer
force-pushed
the
efficient-dir-json
branch
from
e3f509d
to
3633c00
Compare
greatroar
reviewed
May 30, 2022
greatroar
reviewed
May 30, 2022
greatroar
reviewed
May 30, 2022
greatroar
reviewed
May 30, 2022
MichaelEischer
force-pushed
the
efficient-dir-json
branch
2 times, most recently
from
d5f3cfc
to
628b06a
Compare
|
Minor nitpick: TreeBuilder isn't the perfect name for the new type as it doesn't produce trees. TreeJSONMarshaler? TreeJSONer? Otherwise, LGTM. |
|
What about TreeJSONBuilder? I've picked Builder as part of the name in an attempt to convey that it works incrementally. |
|
Sounds good. |
MichaelEischer
force-pushed
the
efficient-dir-json
branch
from
628b06a
to
10d3db1
Compare
MichaelEischer
force-pushed
the
efficient-dir-json
branch
from
10d3db1
to
a636586
Compare
|
Rebased to fix conflicts with #3733. |
There is no real difference between the FutureTree and FutureFile structs. However, differentiating both increases the size of the FutureNode struct. The FutureNode struct is now only 16 bytes large on 64bit platforms. That way is has a very low overhead if the corresponding file/directory was not processed yet. There is a special case for nodes that were reused from the parent snapshot, as a go channel seems to have 96 bytes overhead which would result in a memory usage regression.
That way it is not necessary to keep both the Nodes forming a Tree and the serialized JSON version in memory.
FutureBlob now uses a Take() method as a more memory-efficient way to retrieve the futures result. In addition, futures are now collected while saving the file. As only a limited number of blobs can be queued for uploading, for a large file nearly all FutureBlobs already have their result ready, such that the FutureBlob object just consumes memory.
MichaelEischer
force-pushed
the
efficient-dir-json
branch
from
a636586
to
2ba1416
Compare
|
I've rebased the PR to fix the conflict with #3830 and added a changelog entry |
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.
What does this PR change? What problem does it solve?
restic requires a lot of memory to backup large files and directories. This PR reduces the peak memory usage by about 30-50% (depending on the GC settings). It is not intended as a complete solution to the memory usage problems, as that would require a repository format change as discussed in #2532.
Instead this PR takes the route of reducing the size of structs used during backup. Most notably the FutureNode/File/Tree structs. As the archiver preallocates the FutureNode array for each directory, the optimum would be to reduce the size of each FutureNode to a single reference. As we need a way to wait for the result, this would require a reference to a go channel. However, a channel seems to require about 96 bytes on 64-bit platforms which would regress the memory usage for files that are identical to the parent snapshot. Thus the FutureNode contains an additional reference to the result if it was already available.
The next problem is that to store a Tree in the repository, there are currently up to four items kept in-memory for each directory entry. First there is the FutureNode array, then the Nodes themselves, the serialized JSON of the tree and its encrypted variant while storing these in the repository.
As the FutureNode array is now rather small, it is sufficient to just clear all reference within the FutureNode to allow freeing most memory. The PR introduces a TreeBuilder which incrementally serialized the Tree and thereby ensures that each Node is only kept once in memory: either as Node or its serialized representation.
That way each node is represented in at most two items at a time in memory.
And finally the FutureBlob struct is slimmed down to a single reference to a channel. To reduce the memory usage further, FutureBlobs are collected as soon as possible.
Memory usage measurements:
using go 1.18.2, test data set 500k empty files in a single folder
for i in {1..500000} ; do touch abcefghijklt$i ; donetest command:
/usr/bin/time -v $restic backup -n ../test/hugeThe table reports the results of the first test run, I've repeated the tests another two times, which result in the same overall trends.
The memory usage decreases by roughly 35% and a bit over 50% using GOGC=1. The measurements without
GOGC=1are somewhat unstable as the GC can run at different points in time, but the general trend is stable across repetitions. The memory usage increase foroptimize largeappears to be an outlier as the test case uses no FutureBlobs. The elapsed time increases a bit forGOGC=1which seems to be caused by the different dataset that has to be scanned during GC. WithoutGOGCthe elapsed time seems to be largely unchanged, although it is also too noisy for any further conclusions.Was the change previously discussed in an issue or on the forum?
Related to #2446, but the specific changes were not discussed.
Checklist
[ ] I have added documentation for relevant changes (in the manual).changelog/unreleased/that describes the changes for our users (see template).gofmton the code in all commits.