Skip to content

refactor(memtrack): rename track-rmap toggle to track-physical - #526

Open
not-matthias wants to merge 1 commit into
mainfrom
cod-3468-convert-track-rmap-toggle-into-physical-memory-toggle
Open

refactor(memtrack): rename track-rmap toggle to track-physical#526
not-matthias wants to merge 1 commit into
mainfrom
cod-3468-convert-track-rmap-toggle-into-physical-memory-toggle

Conversation

@not-matthias

Copy link
Copy Markdown
Member

What

Renames the memtrack physical-memory tracking toggle so it names the capability rather than the underlying mechanism:

  • Env var: CODSPEED_MEMTRACK_TRACK_RMAP -> CODSPEED_MEMTRACK_TRACK_PHYSICAL
  • Field: TrackerOptions.rmap -> TrackerOptions.physical

Physical (resident) memory is reconstructed from the folio rmap fentry hooks, so physical describes the intent while the internal rmap mechanism layer (RmapSupport, MemtrackBpf::new_with_rmap, for_each_rmap_*, the BPF C) keeps its accurate name.

Behavior

Unchanged. Tracking stays opt-in (enable with =1); the builder default remains false. The full folio rmap hook set only exists on newer kernels, and RmapSupport::detect() still gates what actually attaches per kernel — so the default is not flipped to on.

Notes

  • Only the env-var/field names change; callers and the rmap_only_options() test helper are updated accordingly.
  • cargo check --tests -p memtrack passes (crate + test targets compile). The privileged rss/c integration tests are GITHUB_ACTIONS-gated and skipped locally, so they were not executed here.

Closes COD-3468

Rename the CODSPEED_MEMTRACK_TRACK_RMAP environment variable and the
TrackerOptions.rmap field to their physical-memory-oriented names
(CODSPEED_MEMTRACK_TRACK_PHYSICAL / TrackerOptions.physical). Physical
(resident) memory is reconstructed from the folio rmap fentry hooks, so
the toggle now names the capability rather than the mechanism.

Default behavior is unchanged: tracking stays opt-in (=1), since the full
folio rmap hook set only exists on newer kernels and RmapSupport::detect
still gates what actually attaches. The rmap mechanism layer keeps its
name.
@not-matthias
not-matthias marked this pull request as ready for review September 3, 2026 13:58
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown

Greptile Summary

The PR renames memtrack’s opt-in physical-memory tracking field and environment variable while retaining the existing rmap-based implementation and default-off behavior.

  • Renames TrackerOptions.rmap to TrackerOptions.physical.
  • Replaces CODSPEED_MEMTRACK_TRACK_RMAP with CODSPEED_MEMTRACK_TRACK_PHYSICAL.
  • Updates the rmap-focused test helper to use the renamed builder option.

Confidence Score: 5/5

The PR appears safe to merge with no concrete regressions identified.

The renamed option remains default-off, is passed unchanged into the existing rmap implementation, and has no stale consumers in the primary or related repositories.

Important Files Changed

Filename Overview
crates/memtrack/src/ebpf/tracker.rs Consistently renames the public option and environment-variable plumbing while preserving defaults and BPF attachment behavior.
crates/memtrack/tests/shared.rs Updates the physical-memory test helper to use the renamed builder setter.

Reviews (1): Last reviewed commit: "refactor(memtrack): rename track-rmap to..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Sep 3, 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 cod-3468-convert-track-rmap-toggle-into-physical-memory-toggle (15002cf) with main (6ecb3c0)

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