Skip to content
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

include the CTxWitness structure in memory usage accounting #269

Conversation

arvidn
Copy link

@arvidn arvidn commented Sep 21, 2017

Also fixed comment in memusage.h

src/memusage.h Outdated
@@ -38,8 +38,9 @@ template<typename X> static inline size_t DynamicUsage(X * const &v) { return 0;
template<typename X> static inline size_t DynamicUsage(const X * const &v) { return 0; }

/** Compute the memory used for dynamically allocated but owned data structures.
* For generic data types, this is *not* recursive. DynamicUsage(vector<vector<int> >)
* will compute the memory used for the vector<int>'s, but not for the ints inside.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment is misleading at best. It does account for the array of ints ("the ints").

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I just realized the example had nested vectors. allright, I'll revert this

@arvidn arvidn force-pushed the account-memusage-of-txwitness branch from 7125df9 to affbfad Compare September 21, 2017 19:51
@@ -26,27 +26,49 @@ static inline size_t RecursiveDynamicUsage(const CTxOut& out) {
return RecursiveDynamicUsage(out.scriptPubKey);
}

//TODO Account for CTxWitness.
static inline size_t RecursiveDynamicUsage(const CTxInWitness& wit) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these static are redundant, I copied them for symmetry, though they should probably be removed.

@arvidn arvidn force-pushed the account-memusage-of-txwitness branch from affbfad to 88d0488 Compare September 21, 2017 20:17
@arvidn arvidn force-pushed the account-memusage-of-txwitness branch from 88d0488 to fb37954 Compare September 21, 2017 20:25
@instagibbs
Copy link
Collaborator

instagibbs commented Sep 22, 2017

tACK fb37954

{
"size": 1,
"bytes": 1933,
"usage": 1424,
"maxmempool": 300000000,
"mempoolminfee": 0.00000000
}

became

{
"size": 1,
"bytes": 1933,
"usage": 8208,
"maxmempool": 300000000,
"mempoolminfee": 0.00000000
}

Follow-on PR could be to account for non-witness issuance data.

@instagibbs instagibbs merged commit fb37954 into ElementsProject:elements-0.14.1 Sep 22, 2017
instagibbs added a commit that referenced this pull request Sep 22, 2017
fb37954 include the CTxWitness structure in memory usage accounting. (Arvid Norberg)
@jtimon jtimon added the 0.14.1 label Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants