Reduce memory requirements for RippleState
#4080
Closed
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
`RippleState` was normally handled as a `shared_ptr`. It appears this was done to avoid exceptions. The `make` function returned a `shared_ptr` and used a `nullptr` to signal an error. There can easily be tens of millions of these objects. While there is runtime overhead in allocating on the heap, this patch is motivated by the space overhead of using a `shared_ptr` as well as the space overhead of allocating on the heap. The overhead of malloc is typically 8-16 bytes. The overhead of a shared_ptr is typically around 48 bytes. This patch replaces the return value of the `make` function with an `optional` and replaces variables that used to hold a `shared_ptr<RippleState>` with a `RippleState`.
Storing the sle means storing some fields twice or keeping references to the data. If the sle is able to be reclaimed, this reduces the memory required to store the RippleState. There was also no reason for this class to have a vtable. The virtual destructor was removed and the class was marked final.
nbougalis
suggested changes
Jan 24, 2022
This information can be retrieved from lowLimit and highLimit instead.
ximinez
approved these changes
Jan 27, 2022
nbougalis
approved these changes
Jan 31, 2022
seelabs
force-pushed
the
ripple-state-mem
branch
from
1fe63d3 to
87ef1be
Compare
HowardHinnant
approved these changes
Feb 9, 2022
thejohnfreeman
approved these changes
Feb 11, 2022
This has become a more specialized class. The new name signals the less generalized nature of the class, and helps people making changes to the class understand the implications.
* Rename PathFindTrustLine.{h,cpp} to TrustLine.{h,cpp}
* Rm old comments
* Rm unused include file
* Additional protection agains instantiating TrustLineBase
seelabs
force-pushed
the
ripple-state-mem
branch
from
bde0471 to
caaedcf
Compare
seelabs
added
the
Passed
|
@mvadari @mtrippled it would be fascinating to run a couple tests to see what effect these patches have on the max stable capacity of a test network |
|
@intelliot This patch only effects |
|
This was merged into 1.9.0 |
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.
High Level Overview of Change
The motivation for these patches is saving space in
RippleState. During path finding many millions ofRippleStateobjects may be created and reducing the size ofRippleStatecan help reduce the memory needed for path finding.There are two patches. The first patch replaces many uses of
shared_ptr<RippleState>withRippleState.RippleStatewas normally handled as ashared_ptr. It appears this was done to avoid exceptions. Themakefunction returned ashared_ptrand used anullptrto signal an error.While there is runtime overhead in allocating on the heap, this patch is motivated by the space overhead of using a
shared_ptras well as the space overhead of allocating on the heap. The overhead of malloc is typically 8-16 bytes. The overhead of ashared_ptr is typically around 48 bytes.
The second patch removes the
slethat was stored in theRippleStateobject. Storing the sle means storing some fields twice or keeping references to the data. If the sle is able to be reclaimed, this reduces the memory required to storethe RippleState.
There was also no reason for this class to have a vtable. The virtual destructor was removed and the class was marked final.
Type of Change