Skip to content

resizable_memory: use mprotect instead of mmap to resize on linux#13

Merged
pknowles merged 2 commits intomainfrom
pknowles/mprotect
Aug 4, 2025
Merged

resizable_memory: use mprotect instead of mmap to resize on linux#13
pknowles merged 2 commits intomainfrom
pknowles/mprotect

Conversation

@pknowles
Copy link
Contributor

@pknowles pknowles commented May 9, 2025

Uses mprotect() instead of mmap() to add the additional pages. I'm amazed how much faster this is. Unmapping is supported too now and should allow the OS to reclaim unused pages.

Before

Mapping 1048576 bytes in increments of 1009 bytes

relative ns/op op/s err% ins/op bra/op miss% total benchmark
100.0% 190,850.05 5,239.72 0.3% 17,428.97 4,422.01 12.0% 12.12 mmap+mprotect
17.3% 1,106,018.39 904.14 0.1% 34,734.03 9,592.02 5.6% 12.07 resizable_memory

Mapping 1048576 bytes in increments of 4096 bytes

relative ns/op op/s err% ins/op bra/op miss% total benchmark
100.0% 190,977.82 5,236.21 0.4% 5,521.96 1,838.01 28.2% 12.06 mmap+mprotect
17.3% 1,104,988.38 904.99 0.2% 16,191.04 4,113.02 12.6% 12.09 resizable_memory

After

Mapping 1048576 bytes in increments of 1009 bytes

relative ns/op op/s err% ins/op bra/op miss% total benchmark
100.0% 192,256.29 5,201.39 0.1% 17,428.97 4,422.01 12.0% 12.09 mmap+mprotect
106.5% 180,528.21 5,539.30 0.1% 28,085.02 7,534.02 7.1% 12.11 resizable_memory

Mapping 1048576 bytes in increments of 4096 bytes

relative ns/op op/s err% ins/op bra/op miss% total benchmark
100.0% 191,424.31 5,224.00 0.2% 5,521.98 1,838.01 28.2% 12.09 mmap+mprotect
106.4% 179,915.29 5,558.17 0.4% 9,806.02 2,837.01 18.2% 12.13 resizable_memory

Summary by CodeRabbit

  • Bug Fixes
    • Improved memory resizing reliability for mapped files on Linux, with better handling of size limits and memory protection.
  • Performance
    • Enhanced efficiency when resizing mapped memory regions by incrementally adjusting protections instead of remapping the entire range.
  • Tests
    • Added new Linux-only tests to verify page residency behavior after decommitting memory.
    • Refactored existing memory resize tests for better isolation and clarity.

@coderabbitai
Copy link

coderabbitai bot commented May 9, 2025

Walkthrough

The MemoryMap and ResizableMappedMemory classes in the Linux-specific header were enhanced for improved memory management. MemoryMap gained a byte_type alias and an address method for offset access. ResizableMappedMemory was refactored to resize memory by incrementally adjusting protections with mprotect and madvise instead of remapping, and its constructor now calls resize directly. Corresponding tests were updated and a new Linux-only test for page residency after decommit was added.

Changes

File(s) Change Summary
Linux MemoryMap and ResizableMappedMemory
include/decodeless/detail/mappedfile_linux.hpp
Added byte_type alias and address method to MemoryMap; simplified MemoryMap::resize to always call mremap without flags and terminate if mapping moves; refactored ResizableMappedMemory to remove map method, call resize in constructor, and implement incremental memory protection changes with mprotect and madvise; replaced size check with throwing std::bad_alloc; updated internal mapped size tracking; moved defaulted move assignment operator below resize.
Mapped Memory Tests
test/src/mappedfile.cpp
Reordered includes; converted ResizeMemory and ResizeMemoryExtended tests from fixture-based (TEST_F) to standalone (TEST) under new MappedMemory suite; added Linux-only PageResidencyAfterDecommit test verifying page residency behavior after commit and decommit using mmap, mprotect, madvise, and mincore; added helper function getResidency to query page residency status.

Possibly related PRs

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

🐇 In Linux lands where pages roam,
We guard their space, we call it home.
With mprotect shields so bright,
Pages wake and rest just right.
From map to resize, smooth and keen,
A rabbit’s joy in code unseen! 🌿✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 599f911 and 977ad2a.

📒 Files selected for processing (2)
  • include/decodeless/detail/mappedfile_linux.hpp (3 hunks)
  • test/src/mappedfile.cpp (4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/src/mappedfile.cpp (1)
include/decodeless/detail/mappedfile_linux.hpp (11)
  • size (75-78)
  • size (75-75)
  • size (141-150)
  • size (141-141)
  • size (214-220)
  • size (214-214)
  • size (231-235)
  • size (231-231)
  • size (262-293)
  • size (262-262)
  • LastError (29-30)
🔇 Additional comments (8)
include/decodeless/detail/mappedfile_linux.hpp (5)

91-91: Good addition of byte_type alias for type safety

The byte_type alias follows the same pattern as address_type and provides proper const-correctness for byte-level operations.


127-129: Clean implementation of offset address calculation

The method provides a type-safe way to calculate offset addresses using the newly added byte_type alias. Good design choice.


256-256: Constructor simplified by delegating to resize()

Good refactor that eliminates code duplication. The resize method now handles all mapping logic in one place.


262-293: Excellent implementation of incremental memory protection changes

The rewritten resize method efficiently uses mprotect() and madvise() instead of remapping, which aligns perfectly with the PR's performance objectives. Key improvements:

  1. Only modifies protection on the specific range being added/removed
  2. Properly aligns to page boundaries
  3. Uses MADV_DONTNEED to actively return physical pages to the OS when shrinking
  4. Maintains both logical size and mapped size tracking

The implementation matches the benchmarks showing ~6x performance improvement.


295-295: Move assignment operator correctly defaulted

Using = default for the move assignment operator is appropriate here since all members support move semantics.

test/src/mappedfile.cpp (3)

230-230: Test organization improved by removing unnecessary fixture dependency

Good refactoring - these memory tests don't require file fixtures, so converting them to standalone tests under the MappedMemory suite is cleaner.

Also applies to: 274-274


431-437: Well-implemented helper for checking page residency

The getResidency function correctly uses mincore to check which pages are resident in memory. Good error handling and proper sizing of the result vector.


439-483: Comprehensive test validates the mprotect+madvise approach

Excellent test that directly validates the page residency behavior of the new implementation:

  • Verifies pages are non-resident initially
  • Confirms pages become resident after writing
  • Validates that mprotect(PROT_NONE) + madvise(MADV_DONTNEED) successfully returns pages to the OS

This test provides confidence that the performance improvements don't compromise memory reclamation.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pknowles/mprotect

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6550dc2 and 2a4947a.

📒 Files selected for processing (1)
  • include/decodeless/detail/mappedfile_linux.hpp (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build (windows-latest, Release, cl)
  • GitHub Check: build (windows-latest, Debug, cl)
🔇 Additional comments (4)
include/decodeless/detail/mappedfile_linux.hpp (4)

257-257: Clean implementation change in constructor

Directly calling resize(initialSize) instead of the separate map() method simplifies the initialization flow. This is more intuitive as it consolidates the memory allocation logic in the resize() method.


283-287: Well-designed protection helper method

The new protect method properly encapsulates the mprotect call with appropriate error handling. The pointer arithmetic is correctly implemented to target the specific memory region.


290-290: Appropriate tracking of mapped memory size

The new m_mappedSize member variable correctly tracks the page-aligned memory size that's actually mapped, which is necessary for the incremental protection approach. This separation from the logical m_size is a good design choice.


250-291: Verify overall impact on memory reclamation

The implementation now properly supports unmapping pages when shrinking the memory region, allowing the OS to reclaim that memory. This should provide memory usage benefits in addition to the performance improvements mentioned in the PR description.

To confirm the memory reclamation behavior is working as expected, you could consider adding a test that:

  1. Allocates a large memory region
  2. Shrinks it significantly
  3. Verifies that the physical memory usage decreases

This would validate that the OS is properly reclaiming the memory when portions are unmapped.

@pknowles pknowles force-pushed the pknowles/mprotect branch from 2a4947a to 74c5514 Compare May 9, 2025 05:37
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a4947a and 74c5514.

📒 Files selected for processing (1)
  • include/decodeless/detail/mappedfile_linux.hpp (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: build (windows-latest, Release, cl)
  • GitHub Check: build (windows-latest, Release, cl)
  • GitHub Check: build (windows-latest, Debug, cl)
🔇 Additional comments (2)
include/decodeless/detail/mappedfile_linux.hpp (2)

257-258: Constructor switch-over to resize() is sound

Calling resize(initialSize) directly from the constructor eliminates the short-lived “partially-initialised” state that existed when we previously relied on a private map() call. The new approach is clearer and guarantees all invariants (m_size, m_mappedSize, protections) are set up in one place.


268-270: 🧹 Nitpick (assertive)

Minor nit – avoid recomputing the page size inside hot paths

pageSize() is already cached, but it is still invoked every time resize() is called. This is fine for most workloads, yet if resize() is on a tight growth loop (e.g. bump allocator), the extra function call shows up in profiles.

-        size_t ps = pageSize();
+        constexpr size_t ps = pageSize();   // forces compile-time constant after first call

Because pageSize() is inline and returns a static value, making the local variable constexpr lets compilers fold it away completely.

Likely an incorrect or invalid review comment.

@pknowles
Copy link
Contributor Author

pknowles commented Aug 4, 2025

Resizing down now actually frees pages. Yes, MADV_DONTNEED does slow things down a little but better to actually free the pages than not. It's also a lot faster than a full remap with mmap(). For the moment, keeping the size as the actual mapped range unlike e.g. std::vector with a separate capacity().

@pknowles pknowles force-pushed the pknowles/mprotect branch from 599f911 to 977ad2a Compare August 4, 2025 00:44
@pknowles pknowles merged commit 977ad2a into main Aug 4, 2025
12 checks passed
@pknowles pknowles deleted the pknowles/mprotect branch August 4, 2025 00:45
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.

1 participant