[cp 1.10] Memory improvements/limits for DelayedFields (#12580, #12583, #12581) #12594
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 reduces memory footprint of amortized usage of delayed field with a single version from 3000 bytes to 600 bytes per delayed field. Basically BTreeMap stores things in vectors of 5, and size of the value is large. This invalidates BTreeMap's cache optimization of storing them all together.
igor-aptos
requested review from
gelash,
runtian-zhou,
sasha8,
danielxiangzl,
zekun000,
davidiw,
movekevin,
wrwg,
vgao1996 and
georgemitenkov
as code owners
igor-aptos
requested review from
sherry-x
and removed request for
davidiw,
wrwg,
movekevin and
sasha8
temporarily, until gas charges for aggregator loading are implemented, limit number of aggregators per resource
igor-aptos
force-pushed
the
igor/blockstm_mem_cherry_pick
branch
from
63ac82d to
907a74f
Compare
vusirikala
approved these changes
Mar 19, 2024
vusirikala
reviewed
Mar 19, 2024
| @@ -246,6 +246,17 @@ impl DelayedFieldValue { | |||
| }, | |||
| }) | |||
| } | |||
|
|
|||
| /// Approximate memory consumption of current DelayedFieldValue | |||
| pub fn get_approximate_memory_size(&self) -> usize { | |||
There was a problem hiding this comment.
because I am not sure about the exact footprint of the vector, and actual size doesn't matter, as there is a lot of uncertainty in the maps that store these as well.
vusirikala
reviewed
Mar 19, 2024
| @@ -211,6 +213,22 @@ pub static BLOCK_COMMITTED_TXNS: Lazy<HistogramVec> = Lazy::new(|| { | |||
| .unwrap() | |||
| }); | |||
|
|
|||
| pub static BLOCK_VIEW_DISTINCT_KEYS: Lazy<HistogramVec> = Lazy::new(|| { | |||
| register_avg_counter_vec( | |||
| "aptos_execution_block_view_distinct_keys", | |||
There was a problem hiding this comment.
What "keys" does this refer to? Comments would be helpful.
There was a problem hiding this comment.
keys in the view, every map (resource/resource_group/modules/delayed_fields has different key), and each is broken down separately
I am going to leave documentation improvements to follow-up PR on main, as this is a cherry-pick to release branch, not the original PR
vusirikala
reviewed
Mar 19, 2024
|
|
||
| pub static BLOCK_VIEW_BASE_VALUES_MEMORY_USAGE: Lazy<HistogramVec> = Lazy::new(|| { | ||
| register_avg_counter_vec( | ||
| "aptos_execution_block_view_base_values_memory_usage", |
There was a problem hiding this comment.
What does "base values" mean here? Comments would be helpful.
There was a problem hiding this comment.
base value / storage version, basically not the versions from writes, but from storage
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## aptos-release-v1.10 #12594 +/- ##
======================================================
Coverage ? 71.7%
======================================================
Files ? 811
Lines ? 185848
Branches ? 0
======================================================
Hits ? 133364
Misses ? 52484
Partials ? 0 ☔ View full report in Codecov by Sentry. |
zekun000
approved these changes
Mar 20, 2024
sherry-x
approved these changes
Mar 20, 2024
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
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.
Combined three memory related changes into this cherry-pick : #12580, #12583, #12581