Page MenuHomePhabricator

Add DynamicMemoryUsage() to LevelDB

Authored by fpelliccioni on Aug 26 2019, 18:32.


Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
rSTAGING8b217943d251: Add DynamicMemoryUsage() to LevelDB
rABC8b217943d251: Add DynamicMemoryUsage() to LevelDB
Test Plan
make check

Run bitcoind with -debug=leveldb and check the log file for the LEVELDB category and the following pattern:
WriteBatch memory usage: db=..., before=...MiB, after=...MiB
Verify that printed sizes (before and after) make sense.

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fpelliccioni created this revision.Aug 26 2019, 18:32
Owners added a reviewer: Restricted Owners Package.Aug 26 2019, 18:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 26 2019, 18:32
fpelliccioni updated this revision to Diff 10961.Aug 26 2019, 20:01

added Linter exception

Fabien requested changes to this revision.Aug 27 2019, 08:55

Your test plan is not testing for the change, please update it.

175 ↗(On Diff #10961)

I don't get why memory usage is a double.

My understanding is that it is an integer division which is casted into a double then printed with 1 decimal.
This decimal value will always be 0 in this case, and you get the overhead of float parsing/printing for no value.

I tested this to see whether I was missing something and couldn't get any value with a non-zero decimal.

This revision now requires changes to proceed.Aug 27 2019, 08:55
fpelliccioni added inline comments.Aug 27 2019, 16:24
175 ↗(On Diff #10961)

Yes, you are right, it is a bug.
I knew this problem beforehand, but I understand that I have to wait for D3948 to be approved to apply the fix.
Maybe I am wrong... I have to improve my understanding about the ABC workflow.

It is fixed here:

(Pedantic: strictly speaking it is not a Cast it is a Floating–integral (implicit) conversion)

fpelliccioni edited the test plan for this revision. (Show Details)Aug 27 2019, 18:50
fpelliccioni edited the summary of this revision. (Show Details)Aug 27 2019, 20:03
Fabien accepted this revision.Aug 27 2019, 20:04
This revision is now accepted and ready to land.Aug 27 2019, 20:04
This revision was automatically updated to reflect the committed changes.