You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(rallocator)!: replace v1 with native v4 and Seismograph state - #797
Replace the original rallocator implementation with the snmalloc-derived native v4 allocator, preserving Seismograph allocation/free recording and adding a cooperative view of allocator internals.
The migration is based on the canonical v4 experiment (snmalloc revision 511e91a) and rebased on main after #764. The performables, recorder, and runtime fixes already landed there are retained; this PR does not replay that earlier work.
Allocator and recording
Keep the public allocator entry points to Rallocator / rallocator!(). Replace v1 domains, heap APIs, tuning state, and snapshot internals rather than providing compatibility shims.
Support Windows and Linux on x64 and ARM64 through the native platform backends.
Preserve allocation/free/reallocation events, caller layouts, actor attribution, and stacks through the existing Seismograph event system. Addresses are correlation keys, not globally unique allocation-lifetime IDs. Caller projection pairs retained records chronologically before restoring their original recorder order. A moved recorded realloc emits its free before releasing the old span.
Bypass publication entirely when allocation recording is disabled. There is no allocation registry, always-on per-operation observation check, sampler worker, or background clock polling. Persistent owner inventory has lifecycle cost.
OS reservations -> shared global backend -> owner-local ranges
-> small slabs / large ranges
Cross-owner frees -> batched returns -> owning endpoint
Cooperative native state
The replacement seismograph_rallocator source uses schema 3. Every persistent owner endpoint is inventoried, including quiet owners created before recording started. Active owners publish copied summaries after accepted recorded operations and after the native core borrow has ended. Returned owners can be inspected under the existing pool lock.
Summaries cover small-class slabs, large ranges, local range and metadata caches, remote-return state, global cached ranges, and virtual reservations. Collection is bounded to 1,024 owners. Publication walks at most 4,096 nodes per owner; returned-owner inspection shares one 4,096-node walk budget across the capture. Missing, older, busy, unavailable, and partial evidence is explicit; an old contributor is not attributed to a new lease holder.
Publication slots and capture buffers use System, not the installed allocator. Borrowed-owner sizing/encoding does not allocate or deallocate.
Monitor
Replace the former Heap internals view with Native v4, immediately left of Allocations:
Compact owner rows without ages or explanatory prose.
Enter-driven subsystem and small-class drilldowns; Escape/Backspace returns one level.
Contextual F1 help for metric meanings and limitations, keyboard/mouse navigation, and wrap-aware scrolling.
A selectable memory view distinguishes allocator reservations from the sparse page-map VA reservation. Committed, resident, and swapped totals are Unknown, not inferred zeroes.
The Allocations view remains available for who allocated/freed what, including individual events and stacks. Source/capture errors remain visible rather than being collapsed into missing data.
Native monitor example
The preview below is a monochrome rendering of the actual TestBackend output from a real installed-v4 recording, not synthetic telemetry. It includes a quiet, unobserved owner alongside current and older observations.
Performance evidence
Earlier pre-rebase measurements compared canonical v4 without telemetry to the same telemetry-compiled candidate with recording off/on. These are not measurements of the final rebased PR binary. Windows x86-64, release/fat LTO, 12 balanced fresh-process samples; recording-on samples omitted stacks.
Workload
Original v4, ns/item
Recording off
Recording on
Off vs. original
Local 16 B
8.67
8.75
155.45
+0.9%
Local 48 B
8.77
8.70
155.78
-0.8%
Local 96 B
8.70
8.83
154.70
+1.5%
Mixed
33.65
37.18
307.56
+10.5%
Resize
61.42
70.44
1160.46
+14.7%
Remote
7.86
8.27
69.59
+5.2%
The disabled path is not claimed to have zero overhead against an allocator that never contained telemetry. Mixed/reallocation workloads retain event-bridge/code-placement overhead. Measurements used a shared laptop, not an isolated host. Warmup initializes publications; repeated-round traversal and snapshot serialization are outside those timings.
Monitor regressions cover every drilldown, mouse selection, narrow-terminal wrapping, End/PgUp/resize scrolling, capture errors, and stable endpoint selection across refresh.
The separately maintained SDK integration passes its observability/runtime/example checks against the rebased allocator revision; SDK changes are not included in this PR.
An initial workspace run hit an allocated-bytes assertion in the existing Seismograph clear-buffer test, followed by mutex-poison cascades. The recorder source is unchanged from main; the isolated test, full Seismograph unit suite, and final default-parallel workspace run subsequently passed.
Boundaries
This is a breaking allocator-internals/API and native-source schema migration. Owner capacities are independent observations, not an atomic heap census or application-live memory. Virtual reservations are not physical residency; incoming queue endpoints do not establish queue depth. New tuning controls and OS residency sampling are out of scope.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The broad unsafe allocator rewrite and platform-specific memory/concurrency implementation require final human review despite substantial regression coverage.
Review effort: Balanced Findings: None
What changed in this PR
Replaces rallocator v1 with the native v4 owner-return allocator, schema-3 Seismograph state capture, and a Native v4 CLI explorer.
Changes:
Reworks allocator internals, platform HALs, recording, and regression coverage.
Introduces bounded native-state encoding and event-lifetime projection.
Replaces the legacy heap monitor with native owner/backend views.
Human review is required for ff638590: the diff removes the public rallocator::config and GlobalRallocator APIs, configured rallocator! forms and supported Cargo features; it also retires the configured-macro/global-allocator integration cases and replaces Seismograph schema 1 with incompatible schema 3. See the allocator API and changed files. A maintainer should explicitly review the breaking contract and test migration; this is not eligible for automated fast-path approval. CI is not fully green, but check state alone is not the reason for this handoff.
Merge current upstream main without rewriting published history. Add baseline AArch64 CPU hints while reusing the 64-bit platform VM and wait backends. Reject unsupported Linux kernel page sizes before reserving memory, sort the manifest, refresh generated READMEs, and explicitly document the native-only Miri boundary. Coverage thresholds and CodeQL alerts remain unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e40ca22b-ef96-48a4-917b-ac308c1b7844
The reason will be displayed to describe this comment to others. Learn more.
Posted by an AI agent
Warning: Incomplete review
I reviewed allocator and native-state public contracts, correctness, tests, performance, naming, telemetry, resilience, and code/documentation consistency. I could not check:
Output-only exported API for rallocator and seismograph_rallocator: matching all-features x86_64 Windows cargo-public-api outputs were unavailable under the read-only restriction.
Tests, Miri, and benchmarks were not run; the correctness and performance observations are source-only. No overall verdict is given. The comments below come from the reviewed areas.
🤖 Updated without rewriting history: upstream merge/ARM64 support is in c02b70b; follow-up 61c6935 repairs generated README drift, adds regressions, and fixes zero-byte report flows producing NaN SVG widths.
Prior Linux/ARM Linux CI passed all 7,175/7,171 tests; their failures were unchanged 100% coverage gates. Local measured coverage improved to allocator 97.3%, schema 98.2%, CLI 98.8%. Strict workspace lint, targeted all-feature tests, workspace/release builds, formatting, spelling, and README checks passed locally.
Still blocked: coverage deficits and 69 CodeQL invalid-pointer findings requiring complete per-flow triage. No checks were weakened or findings dismissed. Fresh checks are running.
This description omits the newly supported ARM64/AArch64 targets. The crate documents Windows x86-64/ARM64 and Linux x86-64/AArch64 support in crates/rallocator/src/lib.rs:6-7, and the package metadata lists all four docs.rs targets.
The reason will be displayed to describe this comment to others. Learn more.
Posted by an AI agent
Warning: Incomplete review
I reviewed public contracts, source correctness, tests, performance, naming, telemetry, resilience, and code/documentation consistency at this head. I could not check:
Output-only exported public API of rallocator and seismograph_rallocator: matching base/head cargo-public-api --all-features outputs for Windows x86_64 were unavailable under the read-only constraint.
No overall verdict is given. These are new findings from the reviewed areas; earlier comments are not repeated. Tests, benchmarks, and Miri were not run.
Add deterministic metadata commit/refill, invalid request, pagemap bounds, remote observation, lease-free lookup, codec inventory and compact UI regressions. Coverage improves without changing production behavior or the 100 percent gates; remaining gaps are still reported by the exact two-configuration check.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e40ca22b-ef96-48a4-917b-ac308c1b7844
This summary omits the supported ARM targets. The crate documents Linux AArch64 and Windows ARM64 support in crates/rallocator/src/lib.rs:6-7, so the workspace README currently understates the allocator's platform support.
The reason will be displayed to describe this comment to others. Learn more.
Posted by an AI agent
Warning: Incomplete review
I reviewed public contracts, source correctness, tests, performance, naming, telemetry, resilience, and code/documentation consistency. I could not check:
Output-only exported public API of rallocator and seismograph_rallocator: matching exact base/head all-features Windows x86_64 cargo-public-api outputs were unavailable under the read-only restriction.
No overall verdict is given. These are new findings from the reviewed areas; previous findings are not duplicated. Runtime tests, Miri, and benchmarks were not run; the review is source-only.
Published fixes in eab634e: (1) deterministic local/foreign usable-size coverage test for the four Linux ARM uncovered lines; (2) regenerated hosted-Linux mutation harness routing core dumps away from the piped Ubuntu crash handler, restoring the original routing afterward; (3) mutation-only --skip filters for the twelve allocator tests that deliberately launch aborting subprocesses, in all three native cargo-mutants configurations. Ordinary tests, coverage, and runtime-analysis selection are unchanged. Other mutated code may still crash, so the runner-level core-dump protection remains relevant. Validation confirmed exactly 12 tests removed from mutation selection (91 ordinary unit tests versus 79 mutation-selected), all 79 selected tests passing, and an actual cargo-mutants run with a passing baseline, two caught mutations and one unviable mutation. No new production allocator behavior or blanket crate exclusion. Fresh hosted checks are starting; the old Linux x64 runtime job remains running. The ARM mutation runner-loss cause remains unproven until usable logs are available.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The cross-platform unsafe allocator and concurrency rewrite requires final human validation, and stale v1 mutation exclusions also remain.
0 open findings
Previously missed (3)
In code that hasn't changed since last review
Remove stale mutation exclusions for deleted tuning telemetry
.cargo/mutants.linux.toml:149
This mutation exclusion targets tuning_telemetry.rs, which this PR removes with the v1 implementation, so it can no longer match a mutant. Remove this stale rule; the adjacent exclusions for the deleted tuning telemetry and medium allocator should be cleaned up at the same time.
Remove stale mutation exclusions for deleted tuning telemetry
.cargo/mutants.toml:137
This mutation exclusion targets tuning_telemetry.rs, which this PR removes with the v1 implementation, so it can no longer match a mutant. Remove this stale rule; the adjacent exclusions for the deleted tuning telemetry and medium allocator should be cleaned up at the same time.
Remove stale mutation exclusions for deleted tuning telemetry
.cargo/mutants.windows.toml:144
This mutation exclusion targets tuning_telemetry.rs, which this PR removes with the v1 implementation, so it can no longer match a mutant. Remove this stale rule; the adjacent exclusions for the deleted tuning telemetry and medium allocator should be cleaned up at the same time.
Published d080ea4 restoring the four fatal-path tests needed by the mutation gate, while retaining the other eight intentional-abort test skips. The prior Linux x64 and ARM64 jobs each missed the same seven fatal-guard mutations; Windows missed the two shared termination guards. The corrected native mutation configuration now catches all nine selected candidates, including the seven previously missed candidates, with its unmutated baseline passing. No production change, mutant exclusion, or gate reduction. All other checks on the previous head passed, including all four platforms coverage/runtime/MSRV/fast checks and CodeQL. Fresh CI is now queued on this correction; auto-merge remains enabled.
The constant-one global reservation mutant fails existing assertions but stalls other Windows allocator tests. Preserve Linux mutation coverage and exclude only this exact Windows candidate, verified caught in an isolated native run.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce582c45-e365-45a2-bc88-6da302a059fd
Published 18c1805 to address the sole remaining Windows x64 mutation failure on d080ea4: 703 candidates, 628 caught, 74 unviable, zero missed, one timeout.
The exact timed-out candidate replaces global_alloc_reserved with address 1. The uploaded diagnostics show nine existing backend tests fail before other parallel allocator tests stall. A targeted native Windows run of that exact candidate against reserved_refill_failures_do_not_publish_ranges_or_accounting passes baseline and catches the mutant in six seconds.
The change excludes only this exact candidate from .cargo/mutants.windows.toml, with evidence documented in the design overview. Candidate discovery verifies that the zero-return mutation stays enabled on Windows and the constant-one mutation stays enabled in the default and Linux configurations. No production code, assertions, ordinary tests, coverage selection, runtime-analysis selection, or required approval gates changed.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The extensive unsafe allocator, platform VM, concurrency, schema, and monitoring changes require final human review despite strong regression coverage.
Inverting the Linux wait predicate spins on immediate EAGAIN replies, bypassing the per-syscall timeout. The exact candidate times out in hosted CI and an isolated native Linux run; retain all other wait candidates and ordinary tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce582c45-e365-45a2-bc88-6da302a059fd
Published 4e5b385 for the newly completed Linux x64 mutation failure on 18c1805: 724 candidates, 649 caught, 74 unviable, zero missed, one timeout.
The exact candidate changes == to != in the Linux futex wait predicate. The changed-word regression then spins on immediate EAGAIN replies; because each syscall returns immediately, the existing per-syscall test timeout cannot bound that loop. Hosted diagnostics show the changed-word and wake-notification tests hang. An isolated native Linux run confirms baseline passes and this exact candidate times out after 15 seconds.
Added only this exact candidate to the existing known-hanging mutation exclusions and documented the evidence. Native candidate discovery retains removal of wait and its other mutations. No production code, ordinary tests, assertions, coverage/runtime-analysis selection, or required approval gates changed.
Exclude both complete crates from mutation testing in default, Linux, and Windows configurations as requested. Preserve ordinary correctness tests, coverage, runtime analysis, and existing mutation safeguards.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce582c45-e365-45a2-bc88-6da302a059fd
Published 624c811 with the requested complete mutation-testing exclusions: both crates/rallocator/** and crates/seismograph_rallocator/** are excluded in all three configurations (default, Linux, Windows), covering every architecture and every source module in those crates.
Verified actual cargo-mutants discovery on native Windows and Linux: both packages produce ZERO candidates under each of the three configurations. This supersedes the earlier narrow timeout-only exclusions. Existing intentional-abort test filters and known-hanging candidate safeguards remain in place. Ordinary tests, coverage, and runtime-analysis selection are unchanged.
Align the Miri metadata comment with the complete allocator mutation exclusions. No manifest settings or executable code changed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ce582c45-e365-45a2-bc88-6da302a059fd
This file contains hidden or 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
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.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
martintmk
added
the
human-review-required
martintmk
left a comment
Summary
Replace the original rallocator implementation with the snmalloc-derived native v4 allocator, preserving Seismograph allocation/free recording and adding a cooperative view of allocator internals.
The migration is based on the canonical v4 experiment (snmalloc revision
511e91a) and rebased onmainafter #764. The performables, recorder, and runtime fixes already landed there are retained; this PR does not replay that earlier work.Allocator and recording
Rallocator/rallocator!(). Replace v1 domains, heap APIs, tuning state, and snapshot internals rather than providing compatibility shims.Cooperative native state
The replacement
seismograph_rallocatorsource uses schema 3. Every persistent owner endpoint is inventoried, including quiet owners created before recording started. Active owners publish copied summaries after accepted recorded operations and after the native core borrow has ended. Returned owners can be inspected under the existing pool lock.Summaries cover small-class slabs, large ranges, local range and metadata caches, remote-return state, global cached ranges, and virtual reservations. Collection is bounded to 1,024 owners. Publication walks at most 4,096 nodes per owner; returned-owner inspection shares one 4,096-node walk budget across the capture. Missing, older, busy, unavailable, and partial evidence is explicit; an old contributor is not attributed to a new lease holder.
Publication slots and capture buffers use
System, not the installed allocator. Borrowed-owner sizing/encoding does not allocate or deallocate.Monitor
Replace the former Heap internals view with Native v4, immediately left of Allocations:
The Allocations view remains available for who allocated/freed what, including individual events and stacks. Source/capture errors remain visible rather than being collapsed into missing data.
Native monitor example
The preview below is a monochrome rendering of the actual TestBackend output from a real installed-v4 recording, not synthetic telemetry. It includes a quiet, unobserved owner alongside current and older observations.
Performance evidence
Earlier pre-rebase measurements compared canonical v4 without telemetry to the same telemetry-compiled candidate with recording off/on. These are not measurements of the final rebased PR binary. Windows x86-64, release/fat LTO, 12 balanced fresh-process samples; recording-on samples omitted stacks.
The disabled path is not claimed to have zero overhead against an allocator that never contained telemetry. Mixed/reallocation workloads retain event-bridge/code-placement overhead. Measurements used a shared laptop, not an isolated host. Warmup initializes publications; repeated-round traversal and snapshot serialization are outside those timings.
Validation
An initial workspace run hit an allocated-bytes assertion in the existing Seismograph clear-buffer test, followed by mutex-poison cascades. The recorder source is unchanged from
main; the isolated test, full Seismograph unit suite, and final default-parallel workspace run subsequently passed.Boundaries
This is a breaking allocator-internals/API and native-source schema migration. Owner capacities are independent observations, not an atomic heap census or application-live memory. Virtual reservations are not physical residency; incoming queue endpoints do not establish queue depth. New tuning controls and OS residency sampling are out of scope.