lmdb: Platform-specific default map size #111
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The current code works fine on x86 platforms since x86-64 uses at least
48-bit of virtual address space. On other 64-bit platforms like aarch64
or riscv64, the minimum allowed virtual address is 39-bit [1] [2].
Current 2**40 allocation will fail:
```
zict/tests/test_lmdb.py:53:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <zict.lmdb.LMDB object at 0x40043bfc10>
directory = '/tmp/test_lmdb-sfepfnw8'
def __init__(self, directory: str):
import lmdb
# map_size is the maximum database size but shouldn't fill up the
# virtual address space
map_size = 1 << 40 if sys.maxsize >= 2**32 else 1 << 28
# writemap requires sparse file support otherwise the whole
# `map_size` may be reserved up front on disk
writemap = sys.platform.startswith("linux")
> self.db = lmdb.open(
directory,
subdir=True,
map_size=map_size,
sync=False,
writemap=writemap,
)
E lmdb.Error: /tmp/test_lmdb-sfepfnw8: Operation not supported
zict/lmdb.py:43: Error
```
Switching to 2**37 on aarch64 and riscv64 should fix the issue.
[1]: https://www.kernel.org/doc/html/v5.8/arm64/memory.html
[2]: https://www.kernel.org/doc/html/latest/riscv/vm-layout.html
hack3ric
added a commit
to hack3ric/archriscv-packages
that referenced
this pull request
Oct 31, 2023
Backported map size fix from dask/zict#111.
jrbourbeau
reviewed
Oct 31, 2023
There was a problem hiding this comment.
cc @crusaderky if you have bandwidth to take a look at this
felixonmars
pushed a commit
to felixonmars/archriscv-packages
that referenced
this pull request
Nov 1, 2023
Backported map size fix from dask/zict#111.
crusaderky
requested changes
Nov 7, 2023
Comment on lines
+56
to
+59
| elif machine in ["x86_64", "x64"]: | ||
| map_size = 2**40 | ||
| elif machine in ["i386", "i686", "x86"]: | ||
| map_size = 2**30 |
There was a problem hiding this comment.
Suggested change
| elif machine in ["x86_64", "x64"]: | |
| map_size = 2**40 | |
| elif machine in ["i386", "i686", "x86"]: | |
| map_size = 2**30 |
Wasn't this already covered by sys.maxsize // 4?
| map_size = 2**40 | ||
| elif machine in ["i386", "i686", "x86"]: | ||
| map_size = 2**30 | ||
| elif machine.startswith("aarch64") or machine.startswith("armv8") or machine.startswith("riscv64"): |
There was a problem hiding this comment.
Suggested change
| elif machine.startswith("aarch64") or machine.startswith("armv8") or machine.startswith("riscv64"): | |
| elif ( | |
| machine.startswith("aarch64") | |
| or machine.startswith("armv8") | |
| or machine.startswith("riscv64") | |
| ): | |
| # x86-64 uses at least 48-bit of virtual address space. On other 64-bit platforms like aarch64 | |
| # or riscv64, the minimum allowed virtual address is 39-bit so 2**40 allocation will fail. |
crusaderky
reviewed
Nov 7, 2023
| map_size = 2**40 | ||
| elif machine in ["i386", "i686", "x86"]: | ||
| map_size = 2**30 | ||
| elif machine.startswith("aarch64") or machine.startswith("armv8") or machine.startswith("riscv64"): |
There was a problem hiding this comment.
What about 32-bit armv8?
aimixsaka
added a commit
to aimixsaka/archriscv-packages
that referenced
this pull request
Apr 29, 2024
- Temporarily vendor fix-map-size.patch to fix rotten - Waiting for dask/zict#111
felixonmars
pushed a commit
to felixonmars/archriscv-packages
that referenced
this pull request
May 6, 2024
- Temporarily vendor fix-map-size.patch to fix rotten - Waiting for dask/zict#111
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current code works fine on x86 platforms since x86-64 uses at least 48-bit of virtual address space. On other 64-bit platforms like aarch64 or riscv64, the minimum allowed virtual address is 39-bit 1 2. Current 2**40 allocation will fail:
Switching to 2**37 on aarch64 and riscv64 should fix the issue.