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

Branch

feature-backport-990353775

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7242
Build 12529: Bitcoin ABC Buildbot (legacy)
Build 12528: arc lint + arc unit

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	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	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)