Use rustix/windows-sys for page size; drop maplit & page_size by joshuamegnauth54 · Pull Request #7876 · RustPython/RustPython

joshuamegnauth54 · 2026-05-16T01:14:37Z

The page_size crate is a simple libc wrapper for Unix and uses winapi-rs for Windows. windows-sys and windows-rs are the modern alternatives for winapi-rs. Both crates are maintained by Microsoft - they're official. Getting the page size for Unix is a simple call. Rustix can do it safely for us. Windows is effectively the same.

Besides Unix and Windows, I also added the page size for wasm32 which the page_size crate did not support. Rust has a built-in API for wasm32's page size, so I didn't add any new dependencies.

Finally, I dropped maplit which is seven years old and only used in one place. Using collect() with a single item iterator is better in this case because Rust can optimize it. maplist called HashMap::insert which would over allocate to amortize future allocs.

Summary by CodeRabbit

Chores
- Removed unused dependencies and enabled an additional feature on a workspace dependency.
Chores
- Added explicit, platform-specific page-size and allocation-granularity helpers and updated the memory-mapping layer so Python-visible PAGESIZE and ALLOCATIONGRANULARITY use those helpers (PAGESIZE behavior changed for wasm32).

coderabbitai · 2026-05-16T01:14:50Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Remove maplit workspace/crate usage and refactor a single-entry HashMap; drop page_size workspace dependency, enable rustix's param feature, and add platform-gated page_size() / alloc_granularity() implementations used by the mmap module.

Changes

Dependency Simplification and Platform Abstraction

Layer / File(s)	Summary
Remove maplit and refactor HashMap construction `Cargo.toml`, `crates/derive-impl/Cargo.toml`, `crates/derive-impl/src/lib.rs`, `crates/derive-impl/src/compile_bytecode.rs`	Removes `maplit` from workspace and `derive-impl`, deletes crate-root `#[macro_use] extern crate maplit;`, and rewrites a single-entry `HashMap` construction from `hashmap!{...}` to `core::iter::once(...).collect()`.
Replace page_size with rustix and add platform-specific implementations `Cargo.toml`, `crates/stdlib/Cargo.toml`, `crates/stdlib/src/mmap.rs`, `crates/host_env/Cargo.toml`, `crates/host_env/src/os.rs`	Removes `page_size` from workspace and `stdlib` target deps, adds `rustix` `param` feature, switches `PAGESIZE`/`ALLOCATIONGRANULARITY` to call `rustpython_host_env::os::{page_size, alloc_granularity}`, implements those functions for Unix (rustix::param), wasm32 (constant), and Windows (GetSystemInfo), and expands a target predicate to include macOS.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

RustPython/RustPython#7710: Overlaps maplit workspace/dependency changes and related derive-impl edits.
RustPython/RustPython#7747: Also touches page_size dependency handling and related stdlib changes.

Suggested reviewers

ShaharNaveh
youknowone

Poem

🐰 I nibbled through Cargo, nudged maplit away,
rustix lent a param, page sizes now say yay.
Unix counts pages, wasm keeps a steady heap,
Windows asks GetSystemInfo before sleep.
Hop, patch, and build — the burrow's code springs to play.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main changes: replacing page_size and maplit dependencies with platform-specific implementations (rustix for Unix, windows-sys for Windows) and removing the dependencies.
Docstring Coverage	✅ Passed	Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

crates/stdlib/src/mmap.rs (1)

198-228: 💤 Low value

Consider extracting the common GetSystemInfo pattern.

Both Windows implementations create a SYSTEM_INFO, call GetSystemInfo, and return a field cast to usize. This pattern could be extracted to reduce duplication.

♻️ Optional refactor

+#[cfg(windows)]
+fn get_system_info() -> SYSTEM_INFO {
+    let mut info = SYSTEM_INFO::default();
+    unsafe {
+        GetSystemInfo(&mut info);
+    }
+    info
+}
+
 #[cfg(windows)]
 #[pyattr(name = "PAGESIZE", once)]
 fn page_size(_vm: &VirtualMachine) -> usize {
-    let mut info = SYSTEM_INFO::default();
-    unsafe {
-        GetSystemInfo(&mut info);
-    }
-    info.dwPageSize as _
+    get_system_info().dwPageSize as _
 }

 #[cfg(windows)]
 #[pyattr(name = "ALLOCATIONGRANULARITY", once)]
 fn granularity(_vm: &VirtualMachine) -> usize {
-    let mut info = SYSTEM_INFO::default();
-    unsafe {
-        GetSystemInfo(&mut info);
-    }
-    info.dwAllocationGranularity as _
+    get_system_info().dwAllocationGranularity as _
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/mmap.rs` around lines 198 - 228, Extract the duplicated
GetSystemInfo pattern into a small helper (e.g., a fn get_system_info() ->
SYSTEM_INFO) and use it from both Windows functions to avoid repeating
allocation and unsafe block: replace the body of page_size and the windows
granularity with calls to this helper and return the appropriate fields
(info.dwPageSize and info.dwAllocationGranularity) cast to usize; keep function
names page_size and granularity and ensure the helper encapsulates the unsafe
GetSystemInfo call and SYSTEM_INFO::default() initialization.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/stdlib/src/mmap.rs`:
- Around line 192-196: The page_size implementation is wrong because
core::arch::wasm32::memory_size(0) returns the current memory page count, not
the fixed page size; change the wasm32 page_size (annotated with #[pyattr(name =
"PAGESIZE", once)] fn page_size) to return the constant WebAssembly page size
65_536 (64 KiB) instead of calling memory_size, and ensure any related symbol
that exposes ALLOCATIONGRANULARITY on wasm32 uses the same 65_536 constant (you
can introduce a WASM_PAGE_SIZE const and return that from page_size and
allocation_granularity).

---

Nitpick comments:
In `@crates/stdlib/src/mmap.rs`:
- Around line 198-228: Extract the duplicated GetSystemInfo pattern into a small
helper (e.g., a fn get_system_info() -> SYSTEM_INFO) and use it from both
Windows functions to avoid repeating allocation and unsafe block: replace the
body of page_size and the windows granularity with calls to this helper and
return the appropriate fields (info.dwPageSize and info.dwAllocationGranularity)
cast to usize; keep function names page_size and granularity and ensure the
helper encapsulates the unsafe GetSystemInfo call and SYSTEM_INFO::default()
initialization.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 359f66fd-62d0-4fe6-9403-f86513e280b7

📥 Commits

Reviewing files that changed from the base of the PR and between ddfcb25 and 8be12df.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

Cargo.toml
crates/derive-impl/Cargo.toml
crates/derive-impl/src/compile_bytecode.rs
crates/derive-impl/src/lib.rs
crates/stdlib/Cargo.toml
crates/stdlib/src/mmap.rs

💤 Files with no reviewable changes (2)

crates/derive-impl/Cargo.toml
crates/derive-impl/src/lib.rs

ShaharNaveh

Awesome!

ty for looking into it:)

youknowone · 2026-05-16T10:03:18Z

Thanks! Could you rebase to to main please?

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/host_env/src/os.rs`:
- Around line 147-176: On Windows there are two problems: the second
`page_size()` should be `alloc_granularity()` and both functions create an
immutable `SYSTEM_INFO` then pass `&mut info` to `GetSystemInfo()`. Fix by
renaming the second function to `alloc_granularity()` and change the local
binding in both `page_size()` and `alloc_granularity()` to `let mut info =
SYSTEM_INFO::default();`, call `unsafe { GetSystemInfo(&mut info); }`, then
return `info.dwPageSize as _` from `page_size()` and
`info.dwAllocationGranularity as _` from `alloc_granularity()`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: d7716633-5c8a-4e64-aa10-af7c2cb6905a

📥 Commits

Reviewing files that changed from the base of the PR and between 1ae5bc9 and 50ba52d.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (7)

Cargo.toml
crates/derive-impl/Cargo.toml
crates/derive-impl/src/compile_bytecode.rs
crates/derive-impl/src/lib.rs
crates/host_env/src/os.rs
crates/stdlib/Cargo.toml
crates/stdlib/src/mmap.rs

💤 Files with no reviewable changes (3)

crates/derive-impl/Cargo.toml
crates/derive-impl/src/lib.rs
crates/stdlib/Cargo.toml

✅ Files skipped from review due to trivial changes (1)

crates/derive-impl/src/compile_bytecode.rs

🚧 Files skipped from review as they are similar to previous changes (1)

Cargo.toml

The `page_size` crate is a simple libc wrapper for Unix and uses `winapi-rs` for Windows. `windows-sys` and `windows-rs` are the modern alternatives for the unmaintained `winapi-rs`. Both crates are maintained by Microsoft - they're official. Getting the page size is a simple call for both Unix and Windows. Besides Unix and Windows, I also added the page size for wasm32 which the `page_size` crate did not support. `wasm32`'s page size is a constant that is defined by the spec, so I hard coded it without adding additional dependencies. Finally, I dropped `maplit` which is seven years old and only used in one place. Calling `collect()` with a single item iterator is idiomatic as well as better in this case because Rust can optimize it. `maplit` called `HashMap::insert` which over allocates to amortize future allocs.

coderabbitai Bot reviewed May 16, 2026

View reviewed changes

Comment thread crates/stdlib/src/mmap.rs Outdated

joshuamegnauth54 force-pushed the drop-some-unused-deps branch from 8be12df to 1ae5bc9 Compare May 16, 2026 01:25

ShaharNaveh approved these changes May 16, 2026

View reviewed changes

joshuamegnauth54 force-pushed the drop-some-unused-deps branch from 1ae5bc9 to 50ba52d Compare May 16, 2026 19:55

coderabbitai Bot reviewed May 16, 2026

View reviewed changes

Comment thread crates/host_env/src/os.rs

joshuamegnauth54 force-pushed the drop-some-unused-deps branch 2 times, most recently from 0bbef59 to 38e1ad3 Compare May 16, 2026 20:07

joshuamegnauth54 force-pushed the drop-some-unused-deps branch from 38e1ad3 to 482bcf3 Compare May 16, 2026 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use rustix/windows-sys for page size; drop maplit & page_size#7876

Use rustix/windows-sys for page size; drop maplit & page_size#7876
joshuamegnauth54 wants to merge 1 commit into
RustPython:mainfrom
joshuamegnauth54:drop-some-unused-deps

joshuamegnauth54 commented May 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 16, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

ShaharNaveh left a comment

Uh oh!

youknowone commented May 16, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

joshuamegnauth54 commented May 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShaharNaveh left a comment

Choose a reason for hiding this comment

Uh oh!

youknowone commented May 16, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joshuamegnauth54 commented May 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 16, 2026 •

edited

Loading