Skip to content

test(memtrack): reproduce fork-inherited unmap removes-exceed-adds - #486

Open
not-matthias wants to merge 1 commit into
mainfrom
test/memtrack-fork-inherited-unmap-contract
Open

test(memtrack): reproduce fork-inherited unmap removes-exceed-adds#486
not-matthias wants to merge 1 commit into
mainfrom
test/memtrack-fork-inherited-unmap-contract

Conversation

@not-matthias

Copy link
Copy Markdown
Member

The rmap resident estimate in the parser is a signed running sum per pid
starting at zero. Fork copies page tables through the folio_*_dup_*
helpers, which the rmap program does not hook, so pages inherited at
fork never produce adds under the child's pid. Unmapping or reclaiming
them fires in-context removes with no matching adds, leaving a
net-negative delta — the root cause behind the negative resident
estimates the parser now floors at zero.

This adds a deterministic, CI-safe repro (no memory pressure needed):
the fixture mmaps and touches 32 MiB, forks a child that munmaps the
inherited region and exits. The test asserts the child's anon rmap net
delta is at least ~-24 MiB with no adds, documenting the event-stream
contract that consumers must tolerate removes exceeding adds per pid.

Companion to the parser-side fix in CodSpeedHQ/platform#2729.

Fork copies page tables through the folio_*_dup_* helpers, which the
rmap program does not hook, so pages inherited at fork never produce
adds under the child's pid. Unmapping or reclaiming them fires
in-context removes with no matching adds, leaving a net-negative delta.
That stream shape is the root cause behind the negative resident
estimates the parser now floors at zero.

The fixture mmaps and touches 32 MiB, forks a child that munmaps the
inherited region and exits; the test asserts the child's anon rmap net
delta is at least ~-24 MiB with no adds. Reproduces in CI without memory
pressure and documents the event-stream contract that consumers must
tolerate removes exceeding adds per pid.
Comment on lines +184 to +186
include_str!("../testdata/rss/rss_report.h"),
)?;
let binary = shared::compile_c_source(source, name, temp_dir.path())?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 RSS test infrastructure is absent

When Cargo compiles the new integration-test target, its compile-time fixture includes, shared::compile_c_source and shared::track_command_with_rmap calls, and RSS event variants are unavailable in the reviewed tree, causing cargo test to fail before the tests can run.

Knowledge Base Used: memtrack: eBPF-based memory allocation tracking

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/memtrack/tests/rss_tests.rs
Line: 184-186

Comment:
**RSS test infrastructure is absent**

When Cargo compiles the new integration-test target, its compile-time fixture includes, `shared::compile_c_source` and `shared::track_command_with_rmap` calls, and RSS event variants are unavailable in the reviewed tree, causing `cargo test` to fail before the tests can run.

**Knowledge Base Used:** [memtrack: eBPF-based memory allocation tracking](https://app.greptile.com/codspeed/-/custom-context/knowledge-base/codspeedhq/codspeed/-/docs/memtrack.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

Adds a deterministic fork-inherited unmap reproduction and an RSS/rmap integration-test target.

  • Allocates and faults 32 MiB before forking, then unmaps the inherited mapping in the child.
  • Asserts that the child's anonymous rmap delta is net-negative by approximately the inherited region size.
  • Introduces broader RSS/rmap aggregation and regression-test helpers.

Confidence Score: 2/5

This PR is not safe to merge until the new RSS integration-test target's missing fixtures and supporting APIs are included or the target is reduced to available infrastructure.

Cargo test discovers the new integration-test file and encounters compile-time includes, helper calls, and event variants that do not exist in the reviewed tree, preventing the test target from compiling.

Files Needing Attention: crates/memtrack/tests/rss_tests.rs

Important Files Changed

Filename Overview
crates/memtrack/testdata/rss/fork_unmap_inherited.c Adds the focused C fixture that faults a private anonymous mapping and unmaps its inherited copy in a child.
crates/memtrack/tests/rss_tests.rs Adds the intended regression assertion alongside a larger test suite, but the new target cannot compile without missing fixtures, helpers, and event variants.

Sequence Diagram

sequenceDiagram
    participant Test as rss_tests
    participant Parent as Fixture parent
    participant Child as Forked child
    participant Rmap as rmap event stream
    Test->>Parent: mmap and touch 32 MiB
    Parent->>Child: fork with inherited page tables
    Note over Rmap,Child: No child add events for inherited pages
    Child->>Rmap: munmap emits child remove events
    Test->>Rmap: Sum child anonymous deltas
    Rmap-->>Test: "Net delta <= -24 MiB"
Loading

Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
### Issue 1
crates/memtrack/tests/rss_tests.rs:184-186
**RSS test infrastructure is absent**

When Cargo compiles the new integration-test target, its compile-time fixture includes, `shared::compile_c_source` and `shared::track_command_with_rmap` calls, and RSS event variants are unavailable in the reviewed tree, causing `cargo test` to fail before the tests can run.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "test(memtrack): reproduce fork-inherited..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Jul 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 17 untouched benchmarks


Comparing test/memtrack-fork-inherited-unmap-contract (934749c) with main (0418479)

Open in CodSpeed

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