Skip to content

Accurately account for mempool index memory #26614

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

Accurately account for mempool index memory #26614

wants to merge 9 commits into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Nov 30, 2022

Picked #18086:

This introduces a C++ allocator class (memusage::AccountingAllocator) which enables containers that accurately account for all their memory allocations in a tracker variable.

This is then used to replace the heuristics in the mempool to guess memory usage.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 30, 2022 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25325 (Add pool based memory resource by martinus)
  • #19909 (refactor: Remove unused CTxMemPool::clear() helper by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@hebasto hebasto mentioned this pull request Nov 30, 2022
Closed
@hebasto hebasto mentioned this pull request Nov 30, 2022
Closed
This was referenced Dec 1, 2022
Merged
Merged
@hebasto hebasto marked this pull request as ready for review December 1, 2022 20:04
@fanquake
Copy link
Member

It would be good for the PR description to outline:

  • What is currently wrong with our memory accounting?
    • How inaccurate is it?
  • How (much more) accurate is it after this change?
    • Is the accuracy affected by things like standard library (version), compiler used etc.
  • How does someone reviewing this PR check the accounted memory usage, before and after?

Maybe the above is all completely obvious from the changes, but I think a PR like this would benefit from having a PR description that isn't just copy-paste from the original 2-year-old PR. Especially since this PR is also missing all the discussion/questions that happened in the original PR (anything relevant could be recapped in the description here).

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 4, 2023

🐙 This pull request conflicts with the target branch and needs rebase.

@hebasto hebasto closed this Mar 21, 2023
@martinus martinus mentioned this pull request May 25, 2023
Closed
@bitcoin bitcoin locked and limited conversation to collaborators Mar 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Mempool Needs rebase Resource usage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hebasto @DrahtBot @fanquake @sipa