Add DynamicMemoryUsage() to LevelDB
ClosedPublic
Actions

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

Details

Reviewers

Fabien
deadalnix

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING8b217943d251: Add DynamicMemoryUsage() to LevelDB
rABC8b217943d251: Add DynamicMemoryUsage() to LevelDB

Summary

Backport of Bitcoin Core PR12604 and PR12784
https://github.com/bitcoin/bitcoin/pull/12604
https://github.com/bitcoin/bitcoin/pull/12784

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

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

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

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7241: Diff 10959.Aug 26 2019, 18:40

added Linter exception

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7242: Diff 10961.Aug 26 2019, 20:08

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

src/dbwrapper.cpp
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

src/dbwrapper.cpp
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: https://github.com/jasonbcox/bitcoin/commit/e80716d3b (Pedantic: strictly speaking it is not a Cast it is a Floating–integral (implicit) conversion)