feat(nvtx): tolerant NVTX model and analyzer — phases 1 & 2 - #472
Closed
9prady9 wants to merge 32 commits into
Closed
feat(nvtx): tolerant NVTX model and analyzer — phases 1 & 2#4729prady9 wants to merge 32 commits into
9prady9 wants to merge 32 commits into
Conversation
Initialize the GSD planning workspace: map the existing codebase, capture project config, research the NVTX ingestion domain, define v1 requirements, and create the 5-phase roadmap linked to GitHub sub-issues rapidsai#371-375. Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Full Phase-1 (NVTX capture foundation) planning and execution record: discuss/research/plan artifacts, per-plan SUMMARYs (events vocabulary, injection cdylib, bridge + capture e2e, full core coverage), tracking updates, code review and security verification, and reconciliation with the upstream merge (PR rapidsai#402). Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase-2 planning artifacts: current-design codebase maps, phase context, NVTX model & tolerant-analyzer research, validation strategy, the 5-plan phase plan and pattern map. Execution started via /gsd-execute-phase and paused at the wave-1 blocking human-verify checkpoint (libc supply-chain legitimacy gate for OS thread-id capture). Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lper - Add thread_id: u32 to NvtxEvent::RangePush and ::RangePop (same OS id space NameThread uses), so the analyzer can reconstruct per-thread stacks - Add libc = "0.2" workspace dep + libc to nvtx-injection deps - Add init::current_thread_id() reading the Linux gettid via SYS_gettid raw syscall (glibc >= 2.30 wrapper unavailable on the CI sysroot), with a std ThreadId-derived non-Linux fallback; capture is Linux-primary - Unit-test current_thread_id() is nonzero and stable within a thread - Construct convert range_pop/range_push/range_push_a with thread_id: 0 as a compile shim; the real id is threaded through in the next task Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
…d-to-end - Thread a thread_id: u32 param through convert::range_pop/range_push/ range_push_a and construct the events with it (convert stays pure) - Read init::current_thread_id() on the app thread inside the catch_unwind guard of all 5 Push/Pop callbacks (CORE2 domain + CORE default-domain) and pass it into convert; wide-char stubs untouched (they emit no event) - Assert thread_id in the convert range_push/range_pop unit tests - Add nvtx-example integration test pushpop_carry_thread_id: every captured RangePush/RangePop carries a nonzero id and Push/Pop from the one example thread share it. Lives in its own tests/thread_id.rs (own process) because install_hook is one-shot per process Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
- Add 02-01-SUMMARY.md (thread_id on Push/Pop, current_thread_id helper, 5 callbacks stamping the OS gettid, end-to-end proof) - STATE.md: advance to plan 2 of 5, phase-02 progress 20%, metrics - ROADMAP.md: mark 02-01 complete, phase 2 at 1/5 In Progress Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
… reconstruction tests - Add `integrations/nvtx/analyzer` (`nvtx-analyzer`), registered in workspace `members` only so default builds stay untouched - Define our own plain span vocabulary: `NvtxSpan`, `SpanKind`, `SpanId` — no `RtFsm`, no macro DSL, no shared analysis/modelling framework dependency - Add a deliberately small `NvtxModelError`/`NvtxModelResult`; stream anomalies are tolerated and logged, never modelled as errors - Stub `NvtxModel`/`NvtxModelBuilder` so tests fail on assertions, not symbols - Add synthetic fixtures plus five RED tests covering handle matching, out-of-order arrival, duplicate timestamps, synthetic close, and orphan ends Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
…Spans - Implement the two-pass `NvtxModelBuilder`: pass 1 materializes the stream in timestamp order via `TimeOrderedCollector`, pass 2 replays and dispatches - Match starts to ends by `range_id` alone — NVTX makes it process-globally unique, so pairs correlate across threads and domains; the end's domain is redundant and ignored - Tolerate malformed streams by construction: orphan ends are logged and skipped, ranges left open close at trace end flagged `synthetic_end`, and inverted pairs clamp to zero duration rather than underflowing - Equal timestamps keep arrival order, so reconstruction is deterministic Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Records the nvtx-analyzer walking skeleton: two-pass builder, Start/End matching by range_id, and the tolerance guarantees, with deviations and verification evidence. Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
…ailing tests - add `tables.rs`: `ResolutionTables` keyed per the two-pass policy — registered strings by `(domain, handle)` and categories by `(domain, category)`, never globally — plus the D-14 placeholder helpers as pure functions of the raw id - add `NvtxMark`, `NvtxDomain`, `NvtxThread`, `NvtxCategory` to `span.rs` - stub the `NvtxModel` accessors so the new tests fail on assertions rather than on missing symbols - add `tests/resolution.rs` with the four RED tests Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
…s, threads, categories - build `ResolutionTables` over the whole ordered stream before replay, so a `RegisterString` arriving after the range that uses its handle still resolves - resolve each `RangeStart` label via `resolve_message(domain, ..)`; `ranges.rs` now receives an already-resolved name instead of emitting a placeholder - reconstruct `Mark` events into `NvtxMark` instants, never zero-length spans - populate `domains()`, `threads()`, `categories()` and add the `category_name` / `thread_name` on-demand resolvers - retain the tables on the model so names with no reconstructed entity still resolve Start/End matching and the tolerance behaviour from plan 02-02 are unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
- add range_push/range_pop fixture constructors carrying thread_id - pushpop_nested_per_thread proves interleaved threads nest independently - pushpop_single_thread proves three-deep parent nesting on one thread - unclosed_closed_at_trace_end proves leaked pushes close synthetic - orphan_pop_skipped proves an empty stack pop is skipped, not fatal - malformed_stream_completes proves the whole core builds with no panic Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
…ed stacks - add PushPopRanges: a Vec<OpenPushSpan> stack per (thread_id, domain), mirroring the injection layer's per-thread, per-domain RANGE_DEPTH grain - capture parent at pop time from the range the pop uncovered - reserve a span slot at push time so a child can name its still-open parent - close leaked pushes at trace end with synthetic_end, nesting preserved - orphan pops warn! and skip; Vec::pop makes an empty stack unreachable as a panic - clamp end = end.max(start) so duration can never underflow Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Records the (thread_id, domain) stack design, the slot-reservation fix that makes SpanId parent references resolvable, and the ANA-03/ANA-05 evidence. Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
- resource_lifespan pins end == 400 and synthetic_end == false, so a (domain, handle) match key cannot pass: it would leak every resource to trace end instead of failing visibly - cover the core/unknown identifier_type split, the unclosed-at-trace-end synthetic close, and the orphan-destroy skip - add resource_create / resource_destroy fixtures; resource_destroy takes no domain because the NVTX event carries none - add NvtxSpan::identifier_type_label (None for range spans) and NvtxModel::resources() so the tests fail on assertions, not symbols Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
- add resource.rs: HashMap<u64 handle, OpenResource>, keyed on the handle and nothing else, because ResourceDestroy carries no domain; the domain is recovered from the create and carried onto the span
- label_identifier_type covers the core nvtxResourceGenericType_t set ((CLASS_GENERIC << 16) | 1..=4, confirmed against the pixi nvtx-c nvToolsExt.h that nvtx-injection's bindgen reads) and passes everything else through as "<identifier_type {n}>" - only the raw integer is ever interpolated, so an unknown type cannot mimic a known one
- wire ResourceCreate/Destroy into pass 2; unclosed resources close at trace end flagged synthetic, orphan destroys warn and skip
- no fabricated resource semantics (D-10): a resource is structurally just NvtxSpan { kind: Resource }
Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
…n, category) - pin each component of the grouping key with a test that a partly-right key would still fail: same name in two domains, and same name under two categories, must not merge - assert marks and resource lifespans are excluded, that a synthetic close counts but stays separately identifiable, and that a zero-duration range contributes 0 rather than being dropped - add StatsKey / RangeStats and a stubbed range_statistics() so the tests fail on assertions rather than on missing symbols - add a range_push_in_category fixture to vary category independently Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
- fold completed PushPop/StartEnd spans into count, total, avg, min and max durations; marks are not spans and resource lifespans are filtered out, since neither measures work - synthetic closes contribute to every figure and also to synthetic_count, so a consumer can tell how much of a group is inferred rather than measured (OQ#2) - BTreeMap rather than HashMap so repeated builds iterate identically, and the first span seeds min/max rather than being compared against a zeroed bound - total uses saturating_add and avg uses checked_div, keeping the fold as total a function as the rest of the reconstruction path Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
- add tests/roundtrip.rs behind a new real-capture-tests feature: runs the actual injection layer over nvtx-example's annotation sequence and reconstructs whatever really comes out, rather than a hand-built stream - asserts the named thread, the "startup" mark as an instant (not a span), "phase-1" as a PushPop span carrying a real thread id, "phase-2" as a StartEnd span, and both ranges present in range_statistics() - nvtx-example and quent-instrumentation are optional regular dependencies because Cargo forbids optional dev-dependencies; nothing in src/ uses them, so plain cargo test -p nvtx-analyzer still builds without pixi, libclang or the nvtx-c headers - single-threaded single capture: NVTX injection is process-global one-shot Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
…p plan Records the OQ#3 resolution (core identifier_type values computed from NVTX_RESOURCE_MAKE_TYPE rather than assumed), the OQ#2 resolution (synthetic spans counted and separately flagged), and 5 auto-fixed deviations - notably that Cargo forbids optional dev-dependencies, so the roundtrip's injection dependency is gated via optional regular deps. Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Signed-off-by: Pradeep Garigipati <pgarigipati@nvidia.com>
Contributor
Author
|
Closing — need to use a clean PR branch that filters out .planning/ and GSD tracking commits. |
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
1 participant
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.
Summary
Phases 1 and 2 of the NVTX consumer (#76):
Phase 1 (capture foundation): New
nvtx-eventscrate with the full NVTX FFI vocabulary (RangePush/Pop,RangeStart/End,Mark,DomainCreate/Destroy,RegisterString,NameCategory,NameThread,ResourceCreate/Destroy);nvtx-injectioncdylib withInitializeInjectionNvtx2, per-thread bounded-ring bridge, and a deterministic in-reponvtx-exampletest app that runs under CI with no GPU. Thread id (gettidon Linux) stamped onRangePush/RangePopevents to enable per-thread stack reconstruction.Phase 2 (tolerant analyzer): New
nvtx-analyzercrate — a hand-written, framework-free reconstruction core with zero dependency onquent-analyzer/quent-model/the Quent DSL. Two-passNvtxModelBuilderreconstructs a capturedEvent<NvtxEventEntity>stream into a labeled, query-ableNvtxModel:range_idprocess-wide;TimeOrderedCollectorfor out-of-order and duplicate-timestamp tolerance; unclosed ranges closed at trace-end withsynthetic_endflag(thread_id, domain)nested stacks with parent capture via reserved slots; orphan pops skipped withwarn!; unclosed pushes closed synthetic(domain, handle), categories keyed(domain, category)(never global); forward-reference safe because pass 1 completes before pass 2; stable<…>placeholders for unresolved handlesNvtxModeltypes with query accessorsResourceDestroycarries no domain);identifier_typelabeled from the corenvtxResourceGenericType_tformula ((class << 16) | index)(name, domain, category); synthetic spans tracked separately; marks and resources excludedTest plan
cargo test -p nvtx-example— 2 tests (capture + thread_id)cargo test -p nvtx-analyzer— 23 unit tests (reconstruction, resolution, push/pop, resources, statistics)pixi run cargo test -p nvtx-analyzer --features real-capture-tests— realnvtx_example::run_captureroundtrip (1 additional test, requires nvtx-c headers via pixi)pixi run cargo clippy --workspace --all-targets --all-features --locked -- -D warnings— cleanpixi run cargo fmt --all -- --check— cleangrep -rc "quent_analyzer\|quent_model\|RtFsm" integrations/nvtx/analyzer/srcreturns 0Known issues / follow-up
Four advisory warnings from code review, all in the injection layer (not the new analyzer crate):
on_range_push_wincrementsRANGE_DEPTHbut never emits aRangePushevent — orphan pops for wide-char range pairsthread_idhashesThreadIdthroughDefaultHashertruncated tou32— hash collision riskRangeStats::countuses plain+= 1whiletotal_durationusessaturating_add(inconsistency)These are tracked for the Phase 3 / follow-up work.
Closes #371, #372
🤖 Generated with Claude Code