kvcache-ai/AgentENV

Fork 0

mirror of https://github.com/kvcache-ai/AgentENV.git synced 2026-11-05 01:17:36 +00:00

[PR #132] [CLOSED] feat: add opt-in dirty-page tracking for memory snapshots #200

New issue

Closed

opened 2026-10-29 14:20:56 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:20:56 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/132
Author: @Hygge-Gezelligheid
Created: 8/6/2026
Status: ❌ Closed

Base: main ← Head: codex/issue-119-dirty-page-tracking

📝 Commits (1)

d165ada feat: add opt-in dirty-page tracking

📊 Changes

8 files changed (+196 additions, -9 deletions)

View changed files

📝 config/default.toml (+6 -0)
📝 config/oss_default.toml (+6 -0)
📝 docs/src/configuration/reference.md (+1 -0)
📝 docs/src/deployment/pvm.md (+2 -0)
📝 src/cfg.rs (+78 -3)
📝 src/sandbox/firecracker/config.rs (+16 -0)
📝 src/sandbox/firecracker/instance.rs (+69 -0)
📝 src/sandbox/firecracker/sandbox.rs (+18 -6)

📄 Description

What

Add an opt-in memory_snapshot.track_dirty_pages configuration that enables Firecracker dirty-page tracking for direct memory snapshot capture.

Pass the option when creating a fresh VM and when restoring a VM from a snapshot.
Persist the option in FirecrackerCommonConfig so the capture behavior follows the sandbox that was launched with it.
Require direct_overlaybd=true when dirty-page tracking is enabled.
Reject dirty-page tracking in PVM mode because this combination has not been tested.
Document the new configuration field, environment variable, defaults, and PVM limitation.

Why

The current mincore-based capture path selects resident pages, including pages that were only read. For workloads with a large read-only resident set, this can copy substantially more memory than the guest actually modified.

Firecracker dirty-page tracking selects pages written after tracking begins. This provides the opt-in behavior requested by #119 while preserving the current behavior by default. The dirty bitmap is cumulative across captures, so a later snapshot can restore pages written both before and after an earlier capture.

Closes #119

Related to #120, but this PR does not implement per-snapshot dirty epochs.

Scope and non-goals

Included:

New memory_snapshot.track_dirty_pages boolean, defaulting to false.
AGENTENV_MEMORY_SNAPSHOT_TRACK_DIRTY_PAGES environment-variable support.
Configuration validation for the supported KVM + direct-overlaybd combination.
Firecracker request propagation for fresh VM creation and snapshot restore.
Backward-compatible persistence through a serde-defaulted common-config field.
Unit tests for configuration parsing, mapping, and Firecracker Unix-socket request bodies.
Configuration and PVM documentation updates.

Non-goals / intentionally excluded:

PVM support before that combination is tested.
Dirty-page tracking through the legacy mem.bin capture path.
Per-capture dirty bitmap clearing or the snapshot-epoch model discussed in #120.
Changes to snapshot manifests, OverlayBD layer formats, or public APIs.
A claim of universal snapshot-performance improvement.

Design and behavior changes

MemorySnapshotConfig::track_dirty_pages is mapped into FirecrackerCommonConfig and persisted with the sandbox. Fresh VM setup and snapshot restore both forward the persisted value to their corresponding Firecracker requests.

The direct capture decision uses either the current global direct_overlaybd setting or the persisted dirty-tracking setting. This prevents a sandbox launched with dirty tracking from falling back to the legacy capture path after a server restart or configuration change.

Configuration validation fails early for unsupported combinations:

track_dirty_pages=true with direct_overlaybd=false is rejected because the legacy path cannot provide the required cumulative dirty-page behavior.
track_dirty_pages=true in PVM mode is rejected with an explicit untested-combination error.

Existing configurations continue to use mincore behavior because the new setting defaults to false.

Compatibility and operations

Public API or generated protocol: N/A; no public API or generated protocol changes.
Configuration or defaults: Adds an optional boolean and environment variable. The default is false, so existing deployments retain current behavior. Enabling it requires KVM and direct_overlaybd=true.
Snapshot manifest, artifact layout, or storage format: Unchanged. The persisted Firecracker common config gains a serde-defaulted field, so existing persisted sandbox configurations remain readable.
Upgrade and rollback: Upgrades preserve existing behavior until the option is enabled. Snapshot artifacts keep the existing format; disable the option before rolling back if the older deployment should not receive the new configuration field.
Host requirements, permissions, ports, or dependencies: No new host dependencies, permissions, or ports. The option is limited to KVM because PVM has not been validated.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

make fmt
# passed

make clippy
# passed

cargo test -p agentenv --lib -- --nocapture
# 692 passed, 0 failed, 4 ignored

git diff --check
# passed

Focused Firecracker socket tests assert that dirty tracking is enabled in both the fresh /machine-config request and the restore /snapshot/load request.

A 512 MiB anonymous guest-memory probe verified:

read checksum after restore: passed
write/verify after restore: passed
s1_restores_A=true
s2_restores_A_and_B=true
DAMON-wait checksum: passed

Selected-byte comparison (selection/correctness diagnostic, not an end-to-end speedup claim):

Workload	mincore selected bytes	dirty selected bytes
quiet	51,740,672	4,382,720
read-only	605,220,864	13,094,912
write	227,663,872	175,906,816
cumulative S1	141,078,528	89,845,760
cumulative S2	237,756,416	176,066,560

The read-only probe selected 97.8% fewer bytes with dirty tracking. In full-page-write ABBA runs, both modes selected about 619 MB, as expected. Sequential snapshot medians were 515.236 ms (mincore) and 517.641 ms (dirty), which did not show a clear regression. Random-write results were underpowered after excluding runs with swap I/O, so this PR does not claim a general performance improvement.

Skipped checks and reasons:

make test-unit: not run; the focused agentenv library suite above was run instead.
Repository Rust integration-test command: not run; the KVM correctness scenarios above were executed with the temporary experiment harness, which is not part of this PR.
make -C services test: no services/ changes.
Generated clients/server regeneration: no schema or generated-code changes.

Risks and reviewer notes

Dirty-page tracking is cumulative: S2 includes pages dirtied before S1 as well as pages dirtied afterward. Per-snapshot epochs remain outside this PR and are related to #120.
Performance benefit depends on workload. Read-heavy resident memory benefits in selected bytes; a workload that dirties nearly every page does not.
PVM is intentionally rejected until it receives equivalent correctness validation.
The legacy mem.bin path is intentionally rejected when dirty tracking is enabled.
Most important review points: configuration validation in src/cfg.rs, persisted setting and Firecracker request propagation in src/sandbox/firecracker/{config,instance}.rs, and capture-path selection in src/sandbox/firecracker/sandbox.rs.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/132 **Author:** [@Hygge-Gezelligheid](https://github.com/Hygge-Gezelligheid) **Created:** 8/6/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/issue-119-dirty-page-tracking` --- ### 📝 Commits (1) - [`d165ada`](https://github.com/kvcache-ai/AgentENV/commit/d165adad51af2beedbec5fcb2952bb5422bb39e5) feat: add opt-in dirty-page tracking ### 📊 Changes **8 files changed** (+196 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `config/default.toml` (+6 -0) 📝 `config/oss_default.toml` (+6 -0) 📝 `docs/src/configuration/reference.md` (+1 -0) 📝 `docs/src/deployment/pvm.md` (+2 -0) 📝 `src/cfg.rs` (+78 -3) 📝 `src/sandbox/firecracker/config.rs` (+16 -0) 📝 `src/sandbox/firecracker/instance.rs` (+69 -0) 📝 `src/sandbox/firecracker/sandbox.rs` (+18 -6) </details> ### 📄 Description ## What Add an opt-in `memory_snapshot.track_dirty_pages` configuration that enables Firecracker dirty-page tracking for direct memory snapshot capture. - Pass the option when creating a fresh VM and when restoring a VM from a snapshot. - Persist the option in `FirecrackerCommonConfig` so the capture behavior follows the sandbox that was launched with it. - Require `direct_overlaybd=true` when dirty-page tracking is enabled. - Reject dirty-page tracking in PVM mode because this combination has not been tested. - Document the new configuration field, environment variable, defaults, and PVM limitation. ## Why The current mincore-based capture path selects resident pages, including pages that were only read. For workloads with a large read-only resident set, this can copy substantially more memory than the guest actually modified. Firecracker dirty-page tracking selects pages written after tracking begins. This provides the opt-in behavior requested by #119 while preserving the current behavior by default. The dirty bitmap is cumulative across captures, so a later snapshot can restore pages written both before and after an earlier capture. ## Related issue Closes #119 Related to #120, but this PR does not implement per-snapshot dirty epochs. ## Scope and non-goals Included: - New `memory_snapshot.track_dirty_pages` boolean, defaulting to `false`. - `AGENTENV_MEMORY_SNAPSHOT_TRACK_DIRTY_PAGES` environment-variable support. - Configuration validation for the supported KVM + direct-overlaybd combination. - Firecracker request propagation for fresh VM creation and snapshot restore. - Backward-compatible persistence through a serde-defaulted common-config field. - Unit tests for configuration parsing, mapping, and Firecracker Unix-socket request bodies. - Configuration and PVM documentation updates. Non-goals / intentionally excluded: - PVM support before that combination is tested. - Dirty-page tracking through the legacy `mem.bin` capture path. - Per-capture dirty bitmap clearing or the snapshot-epoch model discussed in #120. - Changes to snapshot manifests, OverlayBD layer formats, or public APIs. - A claim of universal snapshot-performance improvement. ## Design and behavior changes `MemorySnapshotConfig::track_dirty_pages` is mapped into `FirecrackerCommonConfig` and persisted with the sandbox. Fresh VM setup and snapshot restore both forward the persisted value to their corresponding Firecracker requests. The direct capture decision uses either the current global `direct_overlaybd` setting or the persisted dirty-tracking setting. This prevents a sandbox launched with dirty tracking from falling back to the legacy capture path after a server restart or configuration change. Configuration validation fails early for unsupported combinations: - `track_dirty_pages=true` with `direct_overlaybd=false` is rejected because the legacy path cannot provide the required cumulative dirty-page behavior. - `track_dirty_pages=true` in PVM mode is rejected with an explicit untested-combination error. Existing configurations continue to use mincore behavior because the new setting defaults to `false`. ## Compatibility and operations - Public API or generated protocol: N/A; no public API or generated protocol changes. - Configuration or defaults: Adds an optional boolean and environment variable. The default is `false`, so existing deployments retain current behavior. Enabling it requires KVM and `direct_overlaybd=true`. - Snapshot manifest, artifact layout, or storage format: Unchanged. The persisted Firecracker common config gains a serde-defaulted field, so existing persisted sandbox configurations remain readable. - Upgrade and rollback: Upgrades preserve existing behavior until the option is enabled. Snapshot artifacts keep the existing format; disable the option before rolling back if the older deployment should not receive the new configuration field. - Host requirements, permissions, ports, or dependencies: No new host dependencies, permissions, or ports. The option is limited to KVM because PVM has not been validated. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [ ] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [x] Documentation updated - [x] Benchmarks or performance comparison completed Commands and results: ```text make fmt # passed make clippy # passed cargo test -p agentenv --lib -- --nocapture # 692 passed, 0 failed, 4 ignored git diff --check # passed ``` Focused Firecracker socket tests assert that dirty tracking is enabled in both the fresh `/machine-config` request and the restore `/snapshot/load` request. A 512 MiB anonymous guest-memory probe verified: ```text read checksum after restore: passed write/verify after restore: passed s1_restores_A=true s2_restores_A_and_B=true DAMON-wait checksum: passed ``` Selected-byte comparison (selection/correctness diagnostic, not an end-to-end speedup claim): | Workload | mincore selected bytes | dirty selected bytes | | --- | ---: | ---: | | quiet | 51,740,672 | 4,382,720 | | read-only | 605,220,864 | 13,094,912 | | write | 227,663,872 | 175,906,816 | | cumulative S1 | 141,078,528 | 89,845,760 | | cumulative S2 | 237,756,416 | 176,066,560 | The read-only probe selected 97.8% fewer bytes with dirty tracking. In full-page-write ABBA runs, both modes selected about 619 MB, as expected. Sequential snapshot medians were 515.236 ms (mincore) and 517.641 ms (dirty), which did not show a clear regression. Random-write results were underpowered after excluding runs with swap I/O, so this PR does not claim a general performance improvement. Skipped checks and reasons: - `make test-unit`: not run; the focused `agentenv` library suite above was run instead. - Repository Rust integration-test command: not run; the KVM correctness scenarios above were executed with the temporary experiment harness, which is not part of this PR. - `make -C services test`: no `services/` changes. - Generated clients/server regeneration: no schema or generated-code changes. ## Risks and reviewer notes - Dirty-page tracking is cumulative: S2 includes pages dirtied before S1 as well as pages dirtied afterward. Per-snapshot epochs remain outside this PR and are related to #120. - Performance benefit depends on workload. Read-heavy resident memory benefits in selected bytes; a workload that dirties nearly every page does not. - PVM is intentionally rejected until it receives equivalent correctness validation. - The legacy `mem.bin` path is intentionally rejected when dirty tracking is enabled. - Most important review points: configuration validation in `src/cfg.rs`, persisted setting and Firecracker request propagation in `src/sandbox/firecracker/{config,instance}.rs`, and capture-path selection in `src/sandbox/firecracker/sandbox.rs`. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:20:56 +00:00