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

Compensate for memory peak at flush time #10126

Merged
merged 1 commit into from Mar 31, 2017
Merged

Conversation

sipa
Copy link
Member

@sipa sipa commented Mar 30, 2017

It seems that our conversion from CCoinsCacheDB into a CDBBatch, and the following processing of the batch by LevelDB causes a spike in memory usage. In various experiments (all on x86_64, though), I've determined that this spike is between x1.5 and x1.9.

Take this into account ahead of time before deciding whether to flush or not.

@laanwj laanwj added this to the 0.14.1 milestone Mar 30, 2017
@gmaxwell
Copy link
Contributor

Please tag as 0.14.1.

@theuni
Copy link
Member

theuni commented Mar 30, 2017

@sipa are any of your experiments relatively simple to reproduce? I'd like to step through the high usage spots to get a better feel for what's happening.

@sipa
Copy link
Member Author

sipa commented Mar 31, 2017

@theuni Using the top commit at https://github.com/sipa/bitcoin/commits/repmemusage I've done tests with reindexes and syncs from scratch, killing and restarting bitcoind at certain memory usage points. Then looked at (MAX_RSS - 280MB) / (last_reported_cache=_line).

@laanwj
Copy link
Member

laanwj commented Mar 31, 2017

utACK 7228ce8

@laanwj laanwj merged commit 7228ce8 into bitcoin:master Mar 31, 2017
laanwj added a commit that referenced this pull request Mar 31, 2017
7228ce8 Compensate for memory peak at flush time (Pieter Wuille)

Tree-SHA512: 97e9848410fab061402c85d8440c54a50dd8a0203b2ea194013ea116700a6dc1b4b26b8c5f9c9c68c1f5c6b935c5d6c737437c1911b003d9ff5445c570cd449d
laanwj pushed a commit that referenced this pull request Mar 31, 2017
@morcos
Copy link
Member

morcos commented Mar 31, 2017

@sipa I still object to this PR without at least some increase in the default dbcache. I believe that in the event the mempool is full and there is no loan available then performance will be unnecessarily poor. I think 300 MB (with this new calculation) is approximately the bare minimum we can achieve without likely exceeding it by accident when connecting a block. It is not appropriate for a default.

@laanwj laanwj removed this from For backport in High-priority for review Apr 20, 2017
@sipa sipa deleted the peak_compensation branch June 23, 2017 00:38
codablock pushed a commit to codablock/dash that referenced this pull request Oct 24, 2017
7228ce8 Compensate for memory peak at flush time (Pieter Wuille)

Tree-SHA512: 97e9848410fab061402c85d8440c54a50dd8a0203b2ea194013ea116700a6dc1b4b26b8c5f9c9c68c1f5c6b935c5d6c737437c1911b003d9ff5445c570cd449d
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 20, 2019
7228ce8 Compensate for memory peak at flush time (Pieter Wuille)

Tree-SHA512: 97e9848410fab061402c85d8440c54a50dd8a0203b2ea194013ea116700a6dc1b4b26b8c5f9c9c68c1f5c6b935c5d6c737437c1911b003d9ff5445c570cd449d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants