feat(heapwatch): add crate scaffolding and initial design - #613
Draft
martintmk wants to merge 6 commits into
Draft
feat(heapwatch): add crate scaffolding and initial design#613martintmk wants to merge 6 commits into
martintmk wants to merge 6 commits into
Conversation
Introduce `heapwatch`, a global allocator wrapper that continuously accounts heap usage: live bytes, peak, cumulative bytes allocated and freed, and allocation / free / reallocation counts. This change lands the design ahead of the code so the architecture can be reviewed on its own terms. The crate compiles and is registered in the workspace, but exposes no API yet: - `crates/heapwatch/docs/DESIGN.md` -- the shipped architecture: the measurement boundary, per-thread accumulation with batched publication into process-wide atomics, the accuracy contract and its bounds, peak tracking, re-entrancy, edge cases, and the verification strategy. - `crates/heapwatch/docs/TODO.md` -- ideas deliberately left out of the design, with the reasoning for each. - `crates/heapwatch/src/lib.rs` -- crate-level rustdoc summarising the above and pointing at both documents. Registered in `[workspace.dependencies]` so dependents can pick it up once the implementation lands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: def144ff-f8ea-4dc1-822f-f58665301c4d
✅ Version increments look sufficient
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the new heapwatch crate to the workspace as scaffolding and lands its initial, implementation-agnostic design documentation so the architecture can be reviewed before API/behavior is introduced.
Changes:
- Introduces the
crates/heapwatchcrate with package metadata and crate-level documentation. - Adds detailed design/architecture and “future work” docs for the planned allocator-wrapper mechanism.
- Registers
heapwatchin workspace dependencies and updates lockfile/spellcheck allowlist.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/heapwatch/src/lib.rs | Crate-level rustdoc describing purpose, status, mechanism, and measurement boundary (no API yet). |
| crates/heapwatch/README.md | Generated README mirroring the crate-level docs with project badges and links to design docs. |
| crates/heapwatch/docs/TODO.md | Captures intentionally out-of-scope features/ideas and rationale. |
| crates/heapwatch/docs/DESIGN.md | Full architecture and accuracy/semantics contract for the upcoming implementation. |
| crates/heapwatch/CHANGELOG.md | Seeds the changelog file for the new crate. |
| crates/heapwatch/Cargo.toml | Adds initial crate metadata (0.1.0) and inherits workspace settings/lints/include allowlist. |
| Cargo.toml | Registers heapwatch under [workspace.dependencies] (path + version). |
| Cargo.lock | Adds the new path package entry. |
| .spelling | Allows “heapwatch” and “Heapwatch” for spellcheck. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #613 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 443 443
Lines 42764 42764
=======================================
Hits 42764 42764
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The impact-scoped coverage run (`anvil-llvm-cov`) invokes `cargo llvm-cov nextest --package heapwatch@0.1.0`, and nextest exits 4 on "no tests to run". Because heapwatch is scaffolding -- design documents and crate-level rustdoc, no code and no tests -- it was the sole affected package, so `pr-test` failed on all four platforms. Declare `[package.metadata.coverage-gate] min-lines-percent = 0`, the opt-out the recipe already understands for packages with nothing to instrument, so the affected set resolves to empty and the run skips. The comment marks the section for removal once the implementation lands and the workspace-default 100% gate should apply again. Also from code review: - Add `[package.metadata.docs.rs]`. `lib.rs` declares `#![cfg_attr(docsrs, feature(doc_cfg))]`, but without this section docs.rs never sets the cfg, so feature-gated items would be silently omitted once features exist. - Qualify the peak invariant in `DESIGN.md`. Publishing a commit is two atomics (`CURRENT.fetch_add` then `PEAK.fetch_max`), so a reader that lands between them observes a live total the peak has not caught up with. The invariant holds for completed commits; `peak >= current` is not assertable across a reading. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: def144ff-f8ea-4dc1-822f-f58665301c4d
The previous commit qualified invariant 5 ("Once a commit has
completed, ...") but left the bolded statement in the Peak section
unqualified, so the two sections made different claims about the same
invariant. Qualify the bolded statement identically, and reword the
following paragraph so it elaborates on the qualification rather than
walking back a stronger claim.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: def144ff-f8ea-4dc1-822f-f58665301c4d
martintmk
commented
Jul 28, 2026
martintmk
commented
Jul 28, 2026
Addresses review feedback on the design: - Drop peak entirely. A consumer that samples live bytes can maintain its own maximum, and every other view of one is better derived downstream. Removing it also drops the reset operation and the subtler contract a monotone maximum needs, since a candidate combines a local peak and a total read at different instants and so acquires a persistent bias. Recorded in TODO.md as deferred work. - Replace the process-wide statics with counters owned by the HeapWatch instance, read through a Watch handle. The handle carries no type parameters, so a diagnostic endpoint takes it without the allocator's generics being threaded through. Adds a section explaining which totals a thread's pending block commits to. - Add public-surface snippets for both registration and reading. - Tighten the prose throughout: 3872 to 3121 words despite the new section and examples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 26bd2118-4c88-4d5c-bc3e-d12121a95297
Review raised the case of swapping the system allocator for mimalloc: heapwatch's numbers do not move, because it counts what callers requested and the call pattern is unchanged, yet the memory the process actually holds does move. That invariance is deliberate -- it is what makes heapwatch the control variable -- but the design only implied it, in a passing note that allocator-native counters also report retention and fragmentation. Say it outright in the measurement boundary, and add a TODO item sketching the pairing: an optional trait an inner allocator can implement to report committed bytes on demand, so demand and footprint can be reported side by side. Notes that an allocator instance linked into one library also gives the per-component attribution the OS cannot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 26bd2118-4c88-4d5c-bc3e-d12121a95297
martintmk
commented
Jul 28, 2026
Review asked for a surface with one verb. Committing is now entirely internal -- the churn threshold and thread exit -- so a reader calls stats() and has nothing to sequence before it. Removes flush_thread from the public surface, the explicit-flush arrow from the mechanism diagram, and the flush clause from the concurrency contract and the rustdoc. States why there is no flush operation: it would only affect the calling thread, so tightening a process-wide reading would mean calling it on every thread, which a diagnostic endpoint cannot do -- to tighten a bound that is already documented and small. Reworks the idle-pool caveat in the accuracy section, which previously advised flushing before blocking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 26bd2118-4c88-4d5c-bc3e-d12121a95297
| that binary's Rust heap holds and how much has moved through it — continuously, | ||
| in production, at a cost low enough to leave on. | ||
|
|
||
| It sits among four neighbours: |
Member
There was a problem hiding this comment.
How does this compare to alloc_tracker? Are we reinventing some wheels here? It is not clear why we need yet another memory tracker. If there is something novel this design adds, we should make it clear what that novelty is and why it cannot be solved with existing tools. This is the 2nd seeming duplication of an existing OSS crate I see today - our mission is not to reinvent the universe, so I find this general approach of just adding yet another tool to our already large list of crates dubious (but perhaps a clear motivation here will help).
| Implementation-agnostic — for the API see the crate-level rustdoc, for | ||
| forward-looking ideas see [`TODO.md`](./TODO.md). | ||
|
|
||
| ## What heapwatch is |
Member
There was a problem hiding this comment.
There is a lot of text here about what it is but I do not see text about what the actual use cases or problems to solve are. What is the specific motivation? It is not possible today to judge whether/how the design addresses the problem space.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Introduces
heapwatch, a#[global_allocator]wrapper that continuously accounts heap usage -- live bytes, cumulative bytes allocated and freed, and allocation / free / reallocation counts.This PR lands the design ahead of the code, so the architecture can be reviewed on its own terms rather than through an implementation diff. The crate compiles and is registered in the workspace, but exposes no public API yet. The implementation follows in a separate change.
What is heapwatch
It answers how much, never who. No backtraces, no per-allocation metadata, no pointer-to-size shadow map -- which is what keeps the per-allocation cost to a few non-atomic adds, cheap enough to leave enabled in production.
Each thread accumulates its own totals without synchronization and publishes them in batches into atomic totals owned by the
HeapWatchinstance, once churn since its last publication crosses a compile-time threshold. That removes the atomic read-modify-write per allocation that an exact accounting wrapper pays, in exchange for a bounded, stated inaccuracy of at mostthreshold x live threads-- a bound that does not grow with uptime. Reading goes through aWatchhandle obtained from the instance, and is O(1) in the thread count.Contents
crates/heapwatch/docs/DESIGN.mdcrates/heapwatch/docs/TODO.mdcrates/heapwatch/src/lib.rscrates/heapwatch/Cargo.toml0.1.0, no dependencies.crates/heapwatch/README.mdcargo doc2readmeagainstcrates/README.j2.crates/heapwatch/CHANGELOG.mdCargo.toml/Cargo.lock[workspace.dependencies]..spellingheapwatch/Heapwatch.Notes for reviewers
logo.png/favicon.ico. Each crate has bespoke artwork, so rather than copy another crate's, thehtml_logo_url/html_favicon_urldoc attributes are omitted for now (matchingdata_privacy_coreandmultitude_macros).std-- a deliberate exception to the workspaceno_stdpreference, since the mechanism needs a thread-local with a thread-exit destructor.TODO.md.Validation
Run locally against the workspace:
cargo check --workspace --all-features --all-targets --lockedcargo clippy -p heapwatch --all-features --all-targets --lockedcargo doc -p heapwatch --no-deps --all-featureswithRUSTDOCFLAGS=-D warningscargo fmt -p heapwatch -- --config-path ./unstable-rustfmt.toml --checkcargo sort --check --grouped --workspacecargo heather(license headers)cargo doc2readme --check --lib --template ../README.j2cargo spellcheckcould not run locally (itslibclangbuild dependency is unavailable on this machine); CI covers it.