Skip to content

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

Merged
fanquake merged 1 commit into
bitcoin-core:bitcoin-forkfrom
laanwj:2025-05-revert-mmap-increase
May 28, 2026
Merged

Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"#52
fanquake merged 1 commit into
bitcoin-core:bitcoin-forkfrom
laanwj:2025-05-revert-mmap-increase

Conversation

@laanwj

@laanwj laanwj commented May 8, 2025

Copy link
Copy Markdown
Member

After bitcoin/bitcoin#30039, the number of ldb files created is 16 times smaller. 1000 files with the new default of 32MB is 32GB of database.

If we need more (for good performance), we can increase the default file size again.

This patch seems unnecessary now.

This reverts commit 92ae82c:

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.

…64-bit systems"

After bitcoin/bitcoin#30039, the number of ldb files created is 16 times
smaller. 1000 files with the new default of 32MB is 32GB of database.

If we need more, we can increase the default file size again.

This patch seems unnecessary now.

This reverts commit 92ae82c.

@l0rinc l0rinc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, the change was triggered by one of the synchronization efforts between our fork an the upstream repo google#1265

This revert basically leaves us closer to the original LevelDB implementation - where both env_posix.cc and env_windows.cc have this value, see: https://github.com/search?q=repo%3Agoogle%2Fleveldb%20kDefaultMmapLimit&type=code

ACK fd8f696

Comment thread util/env_posix.cc
// Up to 4096 mmap regions for 64-bit binaries; none for 32-bit.
constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 4096 : 0;
// Up to 1000 mmap regions for 64-bit binaries; none for 32-bit.
constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 1000 : 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, this matches upstream and the current the env_windows.cc version

@laanwj

laanwj commented May 9, 2025

Copy link
Copy Markdown
Member Author

Yes, the motivation here is to limit the amount of divergence from upstream leveldb (especially patches they are extremely unlikely to merge), though this is a harmless, trivial change, so could also keep it i don't feel strongly about it. #50 is worse.

@davidgumberg

Copy link
Copy Markdown

ACK fd8f696

I'd be in favor of just bumping the max file size if/when we start approaching the mmap limit again.

@darosior

darosior commented Sep 9, 2025

Copy link
Copy Markdown
Member

Concept ACK. Since bitcoin/bitcoin#30039 the maximum size of the cache will be 4096 * 32MiB = 131GiB. With this PR the maximum size of the cache will be 32GiB, which is still about twice the on-disk size of the UTxO set.

@darosior darosior left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK fd8f696

@andrewtoth andrewtoth left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fd8f696

Based on #61 (comment), we could have a little over 500 files in the chainstate db now, but will mostly be under.

@fanquake fanquake merged commit a7f9bdc into bitcoin-core:bitcoin-fork May 28, 2026
fanquake added a commit to fanquake/bitcoin that referenced this pull request May 28, 2026
a7f9bdc611 Merge bitcoin-core/leveldb-subtree#52: Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"
a2f531d2d0 Merge bitcoin-core/leveldb-subtree#60: util: use [[fallthrough]] directly
1a166221cf Merge bitcoin-core/leveldb-subtree#61: Disable seek compaction
6bfdb6093b Disable seek compaction
42a5f29aa9 util: use [[fallthrough]] directly
c274b50867 Merge bitcoin-core/leveldb-subtree#53: refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
68740f586f Merge bitcoin-core/leveldb-subtree#59: Fix Clang `-Wconditional-uninitialized` warning
70142e186b Merge bitcoin-core/leveldb-subtree#55: build: Require C++17
d123cf5a83 Effectively, this change
31361bf339 Allow different C/C++ standards when this is used as a subproject.
0711e6d082 Fix Clang `-Wconditional-uninitialized` warning
85665f9547 refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
fd8f69657e Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"

git-subtree-dir: src/leveldb
git-subtree-split: a7f9bdc6114fe6eeb848fda2980fe61d86ff2045
fanquake added a commit to bitcoin/bitcoin that referenced this pull request May 28, 2026
a9ac680 build: remove FALLTHROUGH_INTENDED from leveldb.cmake (fanquake)
4d58c32 build: remove -Wno-conditional-uninitialized from leveldb build (fanquake)
58cdb5c Squashed 'src/leveldb/' changes from ab6c84e6f3..a7f9bdc611 (fanquake)

Pull request description:

  Includes:
  * bitcoin-core/leveldb-subtree#52
  * bitcoin-core/leveldb-subtree#53
  * bitcoin-core/leveldb-subtree#55
  * bitcoin-core/leveldb-subtree#59
  * bitcoin-core/leveldb-subtree#60
  * bitcoin-core/leveldb-subtree#61

ACKs for top commit:
  hebasto:
    ACK a9ac680.
  sedited:
    ACK a9ac680
  andrewtoth:
    ACK a9ac680

Tree-SHA512: dc80a0e5eabd63866b395681935ba47bd3f67292049f2cce77f6bcf9cdd6f3bb9bcf2d87ae836a7af5b3f07fe21a1885697cbae3eb930900e396c2588910e12a
mraksoll4 added a commit to Bitwebissss/bitweb-ver1 that referenced this pull request Jun 3, 2026
8e89537cef Merge bitcoin-core/leveldb-subtree#62: ci: Add LevelDB test workflow
47abe23ff2 ci: run LevelDB tests in GitHub Actions
a7f9bdc611 Merge bitcoin-core/leveldb-subtree#52: Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"
a2f531d2d0 Merge bitcoin-core/leveldb-subtree#60: util: use [[fallthrough]] directly
1a166221cf Merge bitcoin-core/leveldb-subtree#61: Disable seek compaction
6bfdb6093b Disable seek compaction
42a5f29aa9 util: use [[fallthrough]] directly
c274b50867 Merge bitcoin-core/leveldb-subtree#53: refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
68740f586f Merge bitcoin-core/leveldb-subtree#59: Fix Clang `-Wconditional-uninitialized` warning
70142e186b Merge bitcoin-core/leveldb-subtree#55: build: Require C++17
d123cf5a83 Effectively, this change
31361bf339 Allow different C/C++ standards when this is used as a subproject.
0711e6d082 Fix Clang `-Wconditional-uninitialized` warning
ab6c84e6f3 Merge bitcoin-core/leveldb-subtree#58: Initialize file_size to 0 to avoid UB
ad9b1c9893 Initialize file_size to 0 to avoid UB
cad64b151d Merge bitcoin-core/leveldb-subtree#57: doc: fix typos
157ed16be9 doc: fix typos
85665f9547 refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
fd8f69657e Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"

git-subtree-dir: src/leveldb
git-subtree-split: 8e89537cef1144b6b75df83ac53239b79d6e2617
mraksoll4 added a commit to bitweb-project/bitweb that referenced this pull request Jun 4, 2026
8e89537cef Merge bitcoin-core/leveldb-subtree#62: ci: Add LevelDB test workflow
47abe23ff2 ci: run LevelDB tests in GitHub Actions
a7f9bdc611 Merge bitcoin-core/leveldb-subtree#52: Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"
a2f531d2d0 Merge bitcoin-core/leveldb-subtree#60: util: use [[fallthrough]] directly
1a166221cf Merge bitcoin-core/leveldb-subtree#61: Disable seek compaction
6bfdb6093b Disable seek compaction
42a5f29aa9 util: use [[fallthrough]] directly
c274b50867 Merge bitcoin-core/leveldb-subtree#53: refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
68740f586f Merge bitcoin-core/leveldb-subtree#59: Fix Clang `-Wconditional-uninitialized` warning
70142e186b Merge bitcoin-core/leveldb-subtree#55: build: Require C++17
d123cf5a83 Effectively, this change
31361bf339 Allow different C/C++ standards when this is used as a subproject.
0711e6d082 Fix Clang `-Wconditional-uninitialized` warning
ab6c84e6f3 Merge bitcoin-core/leveldb-subtree#58: Initialize file_size to 0 to avoid UB
ad9b1c9893 Initialize file_size to 0 to avoid UB
cad64b151d Merge bitcoin-core/leveldb-subtree#57: doc: fix typos
157ed16be9 doc: fix typos
85665f9547 refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
fd8f69657e Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"

git-subtree-dir: src/leveldb
git-subtree-split: 8e89537cef1144b6b75df83ac53239b79d6e2617
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Jun 28, 2026
0a6aeed build: remove FALLTHROUGH_INTENDED from leveldb.cmake (fanquake)
9b7550e build: remove -Wno-conditional-uninitialized from leveldb build (fanquake)
58cdb5c Squashed 'src/leveldb/' changes from ab6c84e6f3..a7f9bdc611 (fanquake)

Pull request description:

  Includes:
  * bitcoin-core/leveldb-subtree#52
  * bitcoin-core/leveldb-subtree#53
  * bitcoin-core/leveldb-subtree#55
  * bitcoin-core/leveldb-subtree#59
  * bitcoin-core/leveldb-subtree#60
  * bitcoin-core/leveldb-subtree#61

ACKs for top commit:
  hebasto:
    ACK 0a6aeed.
  sedited:
    ACK 0a6aeed
  andrewtoth:
    ACK 0a6aeed

Tree-SHA512: dc80a0e5eabd63866b395681935ba47bd3f67292049f2cce77f6bcf9cdd6f3bb9bcf2d87ae836a7af5b3f07fe21a1885697cbae3eb930900e396c2588910e12a
BigcoinBGC pushed a commit to BigcoinBGC/bigcoin that referenced this pull request Jun 30, 2026
b27d612 build: remove FALLTHROUGH_INTENDED from leveldb.cmake (fanquake)
f2b0e14 build: remove -Wno-conditional-uninitialized from leveldb build (fanquake)
58cdb5c Squashed 'src/leveldb/' changes from ab6c84e6f3..a7f9bdc611 (fanquake)

Pull request description:

  Includes:
  * bitcoin-core/leveldb-subtree#52
  * bitcoin-core/leveldb-subtree#53
  * bitcoin-core/leveldb-subtree#55
  * bitcoin-core/leveldb-subtree#59
  * bitcoin-core/leveldb-subtree#60
  * bitcoin-core/leveldb-subtree#61

ACKs for top commit:
  hebasto:
    ACK b27d612.
  sedited:
    ACK b27d612
  andrewtoth:
    ACK b27d612

Tree-SHA512: dc80a0e5eabd63866b395681935ba47bd3f67292049f2cce77f6bcf9cdd6f3bb9bcf2d87ae836a7af5b3f07fe21a1885697cbae3eb930900e396c2588910e12a
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.

6 participants