Skip to content

Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems #1265

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

l0rinc
Copy link

@l0rinc l0rinc commented Apr 4, 2025

Cherry-picking bitcoin-core@92ae82c and revives #613

…ystems

By default LevelDB will only mmap() up to 1000 ldb files for reading and then fall back
to using file desciptors.

The typical linux system has a 'vm.max_map_count = 65530', so mapping only 1000 files
seems arbitarily small. Increase this value to another arbitrarily small value, 4096.

Original change by Clem Taylor. Ported to LevelDB 1.22 by Wladimir J. van der Laan.
@laanwj
Copy link

laanwj commented May 8, 2025

i'm confused, maybe there is value in changing the default, but why do we patch leveldb here when it could be set from client code:

// Can be set using EnvPosixTestHelper::SetReadOnlyMMapLimit().

@l0rinc
Copy link
Author

l0rinc commented May 8, 2025

Are you suggesting we revert it in Bitcoin instead?

@laanwj
Copy link

laanwj commented May 8, 2025

Ideally yes, looking into it. Sure, upstream leveldb might still want to change this default but i find it hard to argue "why 4096".

Hah i like the commit message:

By default LevelDB will only mmap() up to 1000 ldb files for reading and then fall back
to using file desciptors.

The typical linux system has a 'vm.max_map_count = 65530', so mapping only 1000 files
seems arbitarily small. Increase this value to another arbitrarily small value, 4096.

laanwj added a commit to laanwj/bitcoin that referenced this pull request May 8, 2025
Set the default leveldb mmap limit to 4096 from dbwrapper, before
creating the first leveldb context.

The motivation here is to remove the need for a leveldb patch, see
google/leveldb#1265.
laanwj added a commit to laanwj/bitcoin that referenced this pull request May 8, 2025
Set the default leveldb mmap limit to 4096 from dbwrapper, before
creating the first leveldb context.

The motivation here is to remove the need for a leveldb patch, see
google/leveldb#1265.
@laanwj
Copy link

laanwj commented May 8, 2025

Okay this turned out so much worse than i hoped, on the upside, maybe we can just drop this patch: bitcoin/bitcoin#32447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants