feat(build): opt-in rusty_alloc global allocator feature - #5968
Conversation
Add an off-by-default `rusty-alloc` cargo feature to codewhale-tui and codewhale-cli (#5872) that maps the `#[global_allocator]` static to rusty_alloc (the pure-Rust mimalloc v2.4.5 remake — no C compiler, no build script on that path) instead of the default mimalloc. The issue's sketch pinned `rusty_alloc` and used `rusty_alloc_api::RustyAlloc`; the GlobalAlloc impl actually lives in the `rusty_alloc-api` crate (the crate's documented surface), so that is the dependency — it pulls the rusty_alloc 1.1.6 core transitively at the same version. Default build is unchanged: the mimalloc static compiles exactly as before behind `#[cfg(not(feature = "rusty-alloc"))]`. Build with: cargo build -p codewhale-tui --features rusty-alloc Gates (worktree tmp/wt-5872-rusty-alloc, shared target dir): - cargo fmt --all: clean - cargo check -p codewhale-tui --locked: clean (2m31s) - cargo check -p codewhale-tui --locked --features rusty-alloc: clean (5m10s) - cargo check -p codewhale-cli --locked --features rusty-alloc: clean (2m04s) - cargo clippy --workspace --all-targets --all-features --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or: clean (5m20s) - cargo deny check: advisories ok, bans ok, licenses ok, sources ok (both new crates are MIT; no deny.toml entry needed) - cargo test -p codewhale-tui --lib --locked: 11868 passed, 1 failed, 13 ignored (357s). The failure, remote_control::tests::separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_ids, passes in isolation on rerun (1 passed, 0 failed) and is unrelated to allocator wiring — the lib test target does not compile the changed src/main.rs. Reporting it as the one non-green result of the run. CHANGELOG [Unreleased] entry + sync-changelog.sh + regenerated web/lib/changelog.generated.ts (derive-changelog.mjs idempotent). Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ff200bfe-7c1a-44b0-8681-540a9e2e7ed6) |
|
Claude finished @Hmbown's task in 1m 6s —— View job Review: opt-in
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| # mimalloc v2.4.5 remake — no C compiler, no build script) instead of the | ||
| # default mimalloc. Off by default; the default build is unchanged. | ||
| # Build with: cargo build -p codewhale-cli --features rusty-alloc | ||
| rusty-alloc = ["dep:rusty_alloc-api"] |
There was a problem hiding this comment.
🟡 Rust allocator still requires C toolchain
Enabling rusty-alloc still compiles mandatory mimalloc. Its libmimalloc-sys dependency runs the C build this feature intends to avoid. Builds without a C toolchain still fail.
Prompt for agents
The rusty-alloc feature in crates/cli/Cargo.toml and crates/tui/Cargo.toml only enables rusty_alloc-api. Both crates retain mimalloc as an unconditional dependency, so Cargo still builds mimalloc and libmimalloc-sys, including its C build script, when rusty-alloc is selected. Redesign the allocator features and dependency wiring so a rusty_alloc build excludes mimalloc and its native build dependency while the ordinary default build continues using mimalloc. Update the documented build command if selecting the C-free path requires disabling a default allocator feature, and ensure invalid allocator feature combinations fail clearly.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - The `rusty-alloc` cargo feature on `codewhale-tui` and `codewhale-cli` | ||
| opts the binaries into the `rusty_alloc` global allocator (the mimalloc | ||
| v2.4.5 architecture remade in pure Rust — no C compiler or build script | ||
| on that path) instead of the default mimalloc. It is off by default and | ||
| the default build is unchanged; build with | ||
| `cargo build -p codewhale-tui --features rusty-alloc` (#5872). |
There was a problem hiding this comment.
🔍 Changelog edits bypass merge workflow
Repository policy reserves both changelogs for batched commits on main. Remove these branch changes and regenerate the derived web changelog.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fdc59f3ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # mimalloc v2.4.5 remake — no C compiler, no build script) instead of the | ||
| # default mimalloc. Off by default; the default build is unchanged. | ||
| # Build with: cargo build -p codewhale-cli --features rusty-alloc | ||
| rusty-alloc = ["dep:rusty_alloc-api"] |
There was a problem hiding this comment.
When rusty-alloc is selected, this feature only enables rusty_alloc-api; the unconditional mimalloc dependency at line 51 is still built, and Cargo.lock shows that it pulls libmimalloc-sys, whose dependency on cc invokes the native C build. As a result, the advertised build path for environments without a C toolchain can still fail before using RustyAlloc. Make the allocator dependencies mutually exclusive in both the CLI and TUI manifests while retaining mimalloc as the default.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
As wired, enabling --features rusty-alloc does not actually remove the mimalloc/libmimalloc-sys (C toolchain) build path due to unconditional dependencies and the codewhale-cli -> codewhale-tui -> mimalloc dependency chain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an off-by-default rusty-alloc feature to let the codewhale-tui and codewhale-cli binaries use the pure-Rust rusty_alloc global allocator as an alternative to the current mimalloc default, with accompanying documentation and changelog updates.
Changes:
- Introduces
rusty-allocCargo features incrates/tuiandcrates/cliand switches#[global_allocator]viacfg(feature = "rusty-alloc"). - Adds/pins
rusty_alloc-api = 1.1.6at the workspace level and wires it into the two binary crates. - Updates changelogs and build-performance documentation; regenerates the web changelog module.
File summaries
| File | Description |
|---|---|
| web/lib/changelog.generated.ts | Adds an Unreleased changelog entry for the new allocator feature (generated output). |
| docs/BUILD_PERFORMANCE.md | Documents the new allocator feature as a build/perf-related knob. |
| crates/tui/src/main.rs | Selects the global allocator (mimalloc vs rusty_alloc_api::RustyAlloc) based on rusty-alloc. |
| crates/tui/CHANGELOG.md | Adds an Unreleased entry describing the new rusty-alloc feature. |
| crates/tui/Cargo.toml | Adds the rusty-alloc feature and an optional rusty_alloc-api dependency. |
| crates/cli/src/main.rs | Selects the global allocator (mimalloc vs rusty_alloc_api::RustyAlloc) based on rusty-alloc. |
| crates/cli/Cargo.toml | Adds the rusty-alloc feature and an optional rusty_alloc-api dependency. |
| CHANGELOG.md | Adds an Unreleased entry describing the new rusty-alloc feature. |
| Cargo.toml | Pins rusty_alloc-api = 1.1.6 in workspace dependencies. |
| Cargo.lock | Records the new allocator crates (rusty_alloc, rusty_alloc-api) in the lockfile. |
Review details
Suppressed comments (2)
crates/tui/Cargo.toml:112
mimallocandrusty_alloc-apiare both declared in[dependencies], so enabling--features rusty-allocdoes not avoid compilingmimalloc/libmimalloc-sys(C toolchain viacc). If the feature is meant to remove the C toolchain requirement, gate these allocator dependencies so they’re mutually exclusive based on the feature.
shell-words = "1.1.1"
mimalloc.workspace = true
rusty_alloc-api = { workspace = true, optional = true }
crates/cli/Cargo.toml:52
- Even with
--features rusty-alloc,mimallocis still an unconditional dependency here, so Cargo will still buildlibmimalloc-sys(which depends oncc) and require a C toolchain. To make the feature actually avoid C compilation, gate the allocator dependencies somimallocis only pulled when the feature is not enabled.
semver.workspace = true
tokio.workspace = true
mimalloc.workspace = true
rusty_alloc-api = { workspace = true, optional = true }
- Files reviewed: 9/10 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Opt-in global-allocator swap (#5872): build on rusty_alloc (the pure-Rust | ||
| # mimalloc v2.4.5 remake — no C compiler, no build script) instead of the | ||
| # default mimalloc. Off by default; the default build is unchanged. | ||
| # Build with: cargo build -p codewhale-cli --features rusty-alloc | ||
| rusty-alloc = ["dep:rusty_alloc-api"] |
| # Opt-in global-allocator swap (#5872): build on rusty_alloc (the pure-Rust | ||
| # mimalloc v2.4.5 remake — no C compiler, no build script) instead of the | ||
| # default mimalloc. Off by default; the default build is unchanged. | ||
| # Build with: cargo build -p codewhale-tui --features rusty-alloc | ||
| rusty-alloc = ["dep:rusty_alloc-api"] |
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
PR adds an off-by-default rusty-alloc feature to codewhale-tui and codewhale-cli, switching the global allocator in main.rs to rusty_alloc_api::RustyAlloc while keeping mimalloc as the default. The cfg switch itself is straightforward, but the feature does not actually remove mimalloc from the dependency graph, so the advertised no-C-toolchain goal is not met, and there is no automated coverage for the new feature.
Findings
- [ERROR]
mimallocis still compiled whenrusty-allocis enabled, so the no-C-toolchain goal is not met (crates/cli/Cargo.toml)
The PR documentscargo build -p codewhale-tui --features rusty-allocas avoiding a C compiler/build script, but in bothcrates/cli/Cargo.tomlandcrates/tui/Cargo.tomlthemimallocdependency remains non-optional. Cargo builds themimalloccrate (including its C build script) whenever the package is built, regardless of therusty-allocfeature. The#[cfg(not(feature = "rusty-alloc"))]attribute only removes the#[global_allocator]static, not the dependency itself. The pure-Rust allocator path therefore still requires a C toolchain. Makemimallocoptional and feature-gate it so that the default build still selects mimalloc but arusty-allocbuild can skip it. - [WARNING] No automated CI or test coverage for the new
rusty-allocfeature
The PR lists manualcargo check/cargo buildand clippy commands, but no CI configuration or test is added to keep this feature building. Since the feature is off by default, regressions in the feature-gated global allocator switch could land unnoticed.
Suggestions
crates/cli/Cargo.toml— Makemimallocoptional and add a default feature that enables it (for exampledefault = ["mimalloc-allocator"]andmimalloc-allocator = ["dep:mimalloc"]), then mirror this incrates/tui/Cargo.toml. That keeps today's default build on mimalloc while allowing a true no-C-toolchain build with--no-default-features --features rusty-alloc.
Assessment
The cfg-based allocator selection is simple and correctly defaults to mimalloc, but the core value proposition of the feature—removing the C compiler from the build—is not delivered because mimalloc remains an unconditional dependency. This should be fixed before merging, and CI coverage for the feature should be added.
Advisory review by Codewhale (codewhale review --pr 5968 --post, head 7fdc59f3ae9c46d35a1bd4b6989f5271a23b5fac). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
Closes #5872
What
Adds
rusty_alloc(the pure-Rust mimalloc v2.4.5 remake — no C compiler,no build script on that path) as an off-by-default alternative global
allocator next to the existing mimalloc default, exactly as the issue asks.
rusty_alloc-api = "1.1.6". The issue's sketch pinnedrusty_allocbut usedrusty_alloc_api::RustyAlloc— theGlobalAllocimpl actually lives in the
rusty_alloc-apicrate (the project'sdocumented "start here" surface), so that is the dependency; it pulls the
rusty_alloc1.1.6 core transitively at the same version. Verified oncrates.io: both crates at 1.1.6, MIT, no further transitive deps.
rusty-alloc = ["dep:rusty_alloc-api"]feature added tocodewhale-tuiand
codewhale-cli— the two crates whosesrc/main.rsowns the#[global_allocator]static.rusty_alloc_api::RustyAlloc(the README quick start); without it themimalloc static compiles exactly as before behind
#[cfg(not(feature = "rusty-alloc"))]— the default build is unchanged.docs/BUILD_PERFORMANCE.md(the only allocator-config dochit), CHANGELOG
[Unreleased]entry,sync-changelog.sh, andweb/lib/changelog.generated.tsregenerated viaweb/scripts/derive-changelog.mjs(idempotent; keeps the web drift gategreen).
Build with the feature
Gates (real counts)
cargo fmt --allcargo check -p codewhale-tui --locked(default)cargo check -p codewhale-tui --locked --features rusty-alloccargo check -p codewhale-cli --locked --features rusty-alloccargo clippy --workspace --all-targets --all-features --locked -- -D warnings ...cargo deny checkcargo test -p codewhale-tui --lib --lockedThe one failure,
remote_control::tests::separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_ids,passes on isolated rerun (1 passed / 0 failed) — a load/timing flake, not an
allocator effect: the lib test target does not compile the changed
src/main.rs. Reported rather than hidden.Note
Low Risk
Default allocator and runtime behavior are unchanged; the swap only applies when the feature is explicitly enabled at compile time.
Overview
Adds an off-by-default
rusty-allocCargo feature oncodewhale-cliandcodewhale-tuiso builds can use the pure-Rustrusty_allocglobal allocator (rusty_alloc-api1.1.6) instead of mimalloc, avoiding a C toolchain on that path.Each binary’s
main.rsnow selects the#[global_allocator]static withcfg(feature = "rusty-alloc"); default builds still use mimalloc unchanged. WorkspaceCargo.toml/Cargo.lockpin the optional dependency, and changelog plusdocs/BUILD_PERFORMANCE.mddocument how to enable it (cargo build -p codewhale-tui --features rusty-alloc).Reviewed by Cursor Bugbot for commit 7fdc59f. Bugbot is set up for automated code reviews on this repo. Configure here.