feat(deep-window): trigger bounded deep profiling from runtime metrics - #137
Merged
Conversation
Every PM sample rescanned every completed scope of the run, over a deque that was only ever cleared at session end. A 2s window produces ~19,000 samples and a long run leaves tens of thousands of scopes, so the product ran under a mutex on the collector thread and the deque grew for the life of the process. Resolve a whole drain at once instead: sort the samples and a snapshot of the candidate scopes, then sweep with a min-heap for expiry and an ordered set for the winner, so neither is rescanned per sample. Selection is unchanged - interval contains the timestamp, greatest depth, latest start - and a property test checks the sweep against the original resolver over nested, cross-thread and shared-boundary intervals. Ranking gains the scope instance id as a tertiary key. That is not tidiness: the sweep orders a std::set, and a comparator that ever reported two scopes equivalent would make the set keep one and silently drop the other. Retention is keyed to an event-time watermark the PM engine publishes only when CUPTI reports END_OF_RECORDS with no overflow - the one stop reason that proves the hardware buffer was exhausted. Inferring a boundary from the buffer span instead would be an assumption about capacity, wrong precisely when records were left behind. The watermark is monotonic and never advances on a failed or truncated decode, so a lost batch cannot strand the scopes its leftovers still need. Wall clock would be wrong for the same reason in reverse: a stalled collector lets now run on while undecoded samples wait. The candidate snapshot now includes scopes that are still OPEN. PM drains mid-run, so the scope covering a sample is routinely still running when that sample is decoded. Those samples used to fall back to activeScopeNameId_, which answers what is on top NOW rather than what covered the sample, and cannot represent nesting or cross-thread scopes at all. That fallback is gone: a sample no interval covers is left unattributed rather than credited to whatever happened to be open. A close timestamp is taken before the batch lock can be acquired, so a scope could still look open to a snapshot that had already been given its end. Closes now capture and publish that timestamp under one mutex, and the snapshot caps an open interval at any pending close it finds. Deep window close is the deliberate exception: its end is taken before the engine disarm so the recorded window measures the requested boundary, and its pending close is published after the final drain, which leaves a handful of drain samples - 7 of 18,981 in a 2s window, within 0.7ms - attributed to the window they were collected by but past its recorded end. The hard cap runs on every scope close, not only when PM samples arrive: the watermark is the real bound but only PM publishes one, so a Trace-only run had no limit at all. Eviction is counted every time and logged once, since past the cap every close would otherwise emit a line. It reaches the dashboard through the capability matrix rather than the local log alone, and only when PM rows actually exist - evicting scope history on a run with no PM sampling is a memory backstop firing, not attribution being lost. Not covered: the overflow and COUNTER_DATA_FULL branches have no real-GPU run behind them yet.
A rule needs to watch something only the application knows - tokens, steps,
requests - and neither existing primitive fits. A scope costs two locked
batch pushes and a wire row per iteration, which rules it out of a decode
loop, and being one-per it cannot say that a step produced eight tokens.
auto tokens = gpufl::counter("token"); // once, outside the loop
tokens.add(batchSize); // one relaxed atomic add
The handle holds the slot's address, taken once at registration, so add()
never touches the registry container. It could not do so safely in any
case: reading a deque's size or indexing it races a concurrent
registration, even though the addresses of existing elements stay valid.
Slots live for the process and are never freed, so a handle kept in a
static or held by an embedded host across shutdown()/init() stays valid. A
generation number could not have made that safe on its own - it cannot stop
state being freed while another thread is inside add() - and with permanent
slots there is nothing left to rebind to, so no generation check reaches
the hot path either. What separates sessions instead is a baseline taken at
Initialize: ticks from a previous session, or from while gpufl was down,
belong to neither and are excluded rather than counted twice. Both teardown
paths close the session, including the process-exit one.
add() validates before the atomic, since a single atomic add cannot also
validate: non-positive values and anything above a per-call bound are
dropped. That bound is not overflow protection - the counter is 64-bit and
rates are unsigned deltas, correct across a wrap - it catches a caller
passing something that is not a count, which would quietly poison every
later rate. Names are checked against an explicit ASCII set rather than
std::isalnum, whose answer depends on the host program's locale.
Rejections log once and truncate the offending name, because a bad name
usually fails on every call and tick() in a loop would otherwise write a
line per iteration.
tick(name, n) stays as a convenience wrapper, documented as not for tight
loops: it looks the name up every call, which is the cost the handle
exists to avoid. Python gets a Counter object for the same reason.
Lifecycle tests drive Monitor::Initialize/Shutdown rather than the registry
directly, so deleting the wiring fails a test.
Known gap: gpufl is a static library linked separately into the injection
DLL, the Python extension and any host application, so each holds its own
registry. Counters therefore work in embedded mode but not yet under
`gpufl trace`, where the target ticks one registry and the evaluator would
read another. Fixed separately by moving the registry behind a versioned C
ABI in its own shared runtime.
gpufl is a static library, linked separately into gpufl_inject.dll, the Python extension, and any host application. Each copy held its own registry, so under `gpufl trace -- python server.py` the target would tick one and the injected evaluator would read another: the counter reads as Missing forever, which is exactly the case counters were added for. Counters worked embedded and silently did nothing under the launcher. Move the registry into a small shared runtime and bind every module to it through a versioned C ABI. Only C types cross the boundary - no std:: types, no atomic passed by address, nothing that ties the two sides to one compiler's layout - so add() becomes an indirect call plus a relaxed atomic rather than an inlined one. Measure that in the overhead pass before optimising it. Loaded explicitly by path rather than through an import table. Injection arrives via CUDA_INJECTION64_PATH, so the driver maps gpufl_inject.dll into a target process whose DLL search path has no reason to include our bin directory, and an import entry would simply fail there. The path is derived from THIS module's own location, which works wherever it was loaded from. An already-mapped copy is preferred over any path. Deployment colocates the runtime with each consumer - beside the injection DLL, inside the wheel - so several copies exist on disk, and loading them by full path would map them as separate modules with separate registries: the very split this exists to prevent. Binding to whatever is already loaded makes the number of copies irrelevant, and makes the order between Python import and CUDA init irrelevant too. When no runtime is found the module falls back to its own registry. That is right for an embedded host, which holds the only copy of gpufl in the process, and wrong under injection. CounterProvider::isShared() reports which happened so the rule evaluator can refuse a custom counter rule it cannot honour, rather than reporting a counter that is being ticked as Missing. The tests now read back through the active provider rather than the local registry. Colocating the runtime with the test binary made twelve of them fail, which was the mechanism working: writes went to the runtime's registry while assertions read this module's. They pass both with and without the runtime present. Still open: the cross-module property cannot be proven from inside one executable. It needs the launcher plus Python E2E, which in turn needs the deployment rules that put the runtime beside gpufl_inject.dll and the extension.
The provider resolves the runtime from its OWN module's directory, so the library has to be there. Injection is where this matters: the CUDA driver maps gpufl_inject.dll into the profiled target via CUDA_INJECTION64_PATH, and that process's DLL search path has no reason to include our bin directory. Copies now land beside gpufl_inject.dll, beside gpufl.exe, and beside the Python extension, and the runtime is installed into the wheel. Several copies on disk are harmless: the provider prefers an already-mapped module over any path, so whichever consumer binds first loads it for the process and the rest attach to that one. Verified in a real process rather than by inspection. The Python extension ticks a counter and a separate binder - ctypes loading the runtime and calling only through the C ABI - reads back the same value, which a private per-module registry could not produce. Under the launcher, an injected gpufl_inject.dll resolves the runtime from its own directory inside a foreign process, which is the Windows search-path concern that ruled out an import-table dependency. scripts/counter_cross_module_check.py is that check, kept so it can be re-run on other machines. Still open: nothing on the injected side reads counters yet, so the full target-ticks / evaluator-reads path only closes once the rule evaluator exists.
Rules need readings they can treat as evidence. System metrics publish every 100-500ms while the evaluator runs at ~1ms, so a naive read would count one sample hundreds of times and every rule would fire on its first true reading. Each sample carries two timestamps. Publication time alone can never go stale - it advances forever even during a total stall, which is the condition a rule most needs to catch - so staleness is measured against the source instead. That lets zeros accumulate as fresh evidence and still detects a dead source. An empty window does not mean zero for every metric. A rate over no events is a real 0; a percentile over nothing is not, and publishing 0 ms would read as instantaneous kernels rather than no kernels. Config is validated as a combination, not field by field: stale_after must outlast rate_window + sustained + bucket, or the rule goes stale before it can ever accumulate the evidence to fire. The `custom.` prefix makes a typo a parse error rather than a rule that waits forever for a counter that will never exist. Counter lookup is added to the ABI so a rule naming a counter does not create it - otherwise "never registered" and "registered but idle" become indistinguishable, and they need different answers. Seven of the tests were each confirmed to fail with their own fix reverted.
The two custom-counter tests passed alone and failed in the full suite on Linux. Counter slots are permanent by design, so a name shared with another test makes the result depend on execution order - and where a shared runtime is colocated with the test binary, resetting the local registry does not touch the one the source actually reads. The absence check now goes through the active provider for the same reason: asserting against the local registry would have passed without proving anything.
Turns a metric reading into a bounded window, without letting the rule fire on its own profiling overhead. Blackout and recovery stay distinct. Blackout is "a window is open, discard everything"; recovery is "the window closed, refill the clean epoch". Merged, contaminated samples would prove the workload had recovered and the rule would re-fire on the cost of the last window. Blackout covers any open window, manual or scheduled included, because contamination does not care who opened it. Windows carry a token so a manual window cannot spend the rule's budget, and so a request that the coordinator later turns down costs nothing. An open is serviced on a later beat, so the evaluator asks whether its request is still queued rather than treating "not open yet" as "never going to open". sustained_ms is a span between two observations, not an accumulation, and rearm is one predicate whose direction is validated - a rearm on the wrong side of the operator can never be reached, so the rule would fire once and then wait forever while looking healthy. Two gates, each with its own recorded reason. The engine gate matters most: without it a rule spends its whole budget opening windows that arm nothing. Custom counter rules are refused when the registry is not shared and more than one module is in play, since the target would tick one registry and this evaluator read another. An invalid rule never fails init(): config is parsed there, and failing hard would leave no session and nowhere to record the outcome. Fixed while testing: staleness was checked behind the repeat filter, so a source that died stopped advancing its sequence and was never seen to go stale - the rule sat in Pending on a reading nobody was taking. Nine tests were confirmed to fail with their own fix reverted.
The evaluator existed but nothing drove it. GPUFL_DEEP_WHEN now installs a rule at init, the collector beat advances it beside the window service, and the launch callback and sampler feed it. Windows carry the comparison that caused them. A bare observed value stops being readable the first time somebody edits the threshold, so the rule id, operator, threshold, rate window, sustained duration and first-true timestamp travel with the window. Manual windows carry no trigger at all rather than an all-zero one that reads like a real rule. Every rule emits a summary, including one that never fired. Absence has to keep meaning "the run ended unexpectedly" - if a rule that was never true also left nothing, the UI could not tell the two apart. An invalid rule is installed as refused rather than dropped, so it still reports at shutdown, and init always succeeds: config is parsed there, so failing hard would leave nowhere to record the refusal. The launch feed is opt-in per rule. A rule watching a custom counter or a GPU gauge never reads it, and every launch in the process would otherwise pay for a feed nobody consumes. Device count is deliberately not guessed. Devices are not enumerated until the sampler's first measurement, and assuming one would refuse a valid gpu[3] rule on a four-GPU host; a device that never reports is named in the summary instead.
Counter add is lock-free again. A handle now IS the slot's address, so add is one relaxed atomic; it was going through a slot id, which had to be bounds-checked, which meant locking the registry - putting every ticking thread on one mutex and distorting the throughput a rule exists to measure. The header claimed this contract all along. The pending open is one record. Spec and owner token were separate globals, so an untagged request arriving after a tagged one replaced only the spec: a manual window then opened carrying a rule's token and was charged to that rule's budget, which is the exact mis-attribution the token was added to prevent. Custom metrics read since the session baseline. Slots are permanent, so a counter ticked by a previous session made a new session believe it had already moved, and a stall rule could arm on evidence this run never saw. The launch callback takes no lock at all. It held the rule mutex and then the feed mutex, on the application's launch path, contending with the collector's polling. Finish runs after the collector is joined, and freezes the evaluator first; it could previously write a summary and then let a window open that the summary does not mention. It also releases the session before attempting the write, so a shutdown with no logger no longer leaves the session claimed and the next init() silently rule-less. recent_kernel_ms is fed. Only launches were wired, so a rule watching kernel duration read nothing and ended every run as missing. Window detection uses the monotonic open counter. A launch-bounded window can open and close between two beats, and a boolean poll would miss it entirely - its contaminated samples then feeding the rule as clean. Malformed numeric options fail closed instead of silently defaulting, capability uses the resolved engine rather than the requested one, and the runtime installs unconditionally rather than only with the Python bindings - a launcher-only install left the injected evaluator and the target on separate fallback registries. The cross-module script now exits non-zero on a wrong value, resolves the library per platform, and runs under CTest.
exhausted and unsupported are conclusions a run reaches long before it ends. Holding them until shutdown meant a process that crashed afterwards explained nothing, and the session read as one whose rule simply never fired. The shutdown summary still follows and always carries a higher state_sequence, so the backend's strictly-greater upsert keeps the final row and treats a redelivery of either as a no-op.
It was registered in tests/, which CMake processes before _gpufl_client exists - so its guard was always false and the check silently never registered. Moved to where the target is defined. Running it then exposed two more ways it could have passed without proving anything. It used whichever interpreter find_package picked, which was a different minor version than the extension was built for, so gpufl loaded its stub and ticked nothing. And the stub guard asked whether gpufl exposes counter() - which the stub does. It now pins the interpreter the extension was built against and checks the extension module is actually loaded. The package is staged into the build tree rather than run from source: the extension has to sit inside the package to be importable, and copying a build artifact into source would leave it there afterwards. Verified by removing the runtime library: the check fails, and passes again when it is restored.
…bounds A C++ target could never reach the shared runtime. It links gpufl statically, so the provider's own module directory is the TARGET's directory, which holds no runtime; a target that calls counter() before its first CUDA call - entirely normal - bound to a local registry and stayed there, invisible to the evaluator that loaded later. Resolution now also looks beside CUDA_INJECTION64_PATH, which the launcher puts in the child environment before exec, and honours an explicit GPUFL_COUNTER_RUNTIME_PATH. A failed resolution is no longer cached as final, since the runtime legitimately appears after injection. Pending opens are now first-wins rather than newest-wins. With both --deep-after and a rule configured, the old policy let whichever ran second silently cancel the other - decided by call order rather than by anything the user asked for. A refused rule retries; a cancelled scheduled window was simply gone. recent_kernel_ms is stamped with when the kernel ENDED. Freshness is measured against the source event, so a two-second kernel reported with its start time arrived already two seconds old: the slower the kernel, the more certainly a rule watching for slow kernels discarded it. The duration feed is bounded at the push. Trimming after a drain bounded nothing between drains, and it grows fastest exactly when the drain is late. A long collector stall now discards the feed as well as the local buckets, so durations from before the gap cannot fire a rule as if they were current. Numeric options reject ERANGE, and the derived stale-after default is summed with overflow checks - a saturated LLONG_MAX fed into that sum is undefined behaviour rather than merely a wrong number. Four of the five are pinned by tests confirmed to fail with the fix reverted. The ERANGE check is not: the range validators downstream reject saturated values too, so removing it changes which reason is reported rather than whether the rule is refused.
First-wins broke under concurrency. The queued flag was published after the lock was released, so a second caller could take the lock, see nothing queued, and overwrite the first request - defeating the exact rule the check exists to enforce. Published under the lock now, on both the tagged and the scheduled path. Durations carry their own timestamps and are drained up to the closing boundary. A collector that fell behind and closed several boundaries in one poll put the whole backlog into the OLDEST bucket and left the rest empty, which skewed the percentile and expired samples earlier than their own timestamps allowed. Truncated buckets are counted and readable. The counter existed and nothing read it, so a percentile computed from a subset was presented as complete. Reported rather than used to suppress the metric: at the launch rates that cause truncation - hundreds of thousands per second - suppression would disable the metric on exactly the workloads it is for. max_windows is range-checked before narrowing. 4294967297 survives ERANGE, narrows to 1, and the validator accepts it, so the run silently used a budget nobody configured. A failed runtime discovery no longer rescans paths on every counter(). It retries only the cheap already-loaded probe, which is the one thing that can actually change - the injected library maps at the first CUDA call. Coverage, stated plainly: the bucket partition, the truncation count and the max_windows check are each pinned by a test confirmed to fail with the fix reverted. The publish-under-lock ordering is NOT. That window is a few instructions wide and 400 gated rounds never reproduced it; the concurrency test pins the single-winner invariant instead, and fails if the first-wins check is removed. The ordering is correct by construction rather than by demonstration. Also drops cursor.json, a zero-byte file committed by accident.
The previous commit claimed this and did not contain it: cleaning up after a mutation run with 'git checkout -- deep_window.cpp' reverted the uncommitted fix along with the mutation, and the commit message was written from intent rather than from the diff. The fix itself is unchanged - the queued flag is published inside g_mu on both the tagged and the scheduled path, so a second caller can no longer take the lock, see nothing queued, and overwrite the first request.
…l data TakePendingOpen_ cleared the queued flag and only then took the lock. A producer could acquire the lock in that gap, see nothing queued, and store its own request - which the collector then consumed believing it was the earlier one, while the producer's flag stayed set and let the same request open a second window later. The flag and the record are now claimed in one critical section. Truncation reaches the conclusion instead of sitting in a counter. The accessor added last round had exactly one reader: a test. A percentile over part of the data looks identical to one over all of it, so the count now travels on the sample, into the rule summary, onto the wire, and into the UI as a "Partial data" badge. Counted rather than used to suppress the metric - truncation begins well below the launch rates these workloads reach, so suppressing would disable it where it matters most. cursor.json is untracked; the previous removal staged the delete and did not survive into the commit.
…poll Two caps exist and the bucket's is the smaller, so a batch between them - the common case - passed the feed untouched and was then trimmed when the bucket closed, with nothing counting the loss. The reading said the percentile was complete while a fifth of the kernels were gone. The count was also read before the buckets closed, so a loss caused by one poll was only admitted by the next. bucket_truncated_ is gone. It was written and never read - the same 'counter nobody consumes' shape that hid this in the first place - and counting the samples properly makes it redundant. Both halves are pinned by tests that target the gap between the two caps, confirmed to fail with each fix reverted.
They cannot be combined, and the reason is a hardware constraint rather
than a style choice: cuptiProfilerInitialize has to run before any CUDA
context exists, so the deep engines are fixed at process start whatever
the trigger later does. A --passes list therefore either already holds
what the window would arm, making the flag redundant, or does not -
which is what `--passes=Trace --deep-after=30s` silently did, opening a
window that armed nothing and reporting no_deep_engine.
Rejected before the target is launched, in either flag order, with the
way out named. --deep-after is included with no grandfather clause:
"--deep-when refuses but --deep-after tolerates" is harder to explain
than the break, and the replacement selects the same engine on this GPU.
Adds --deep-when, which existed only as an environment variable, so the
rule is reachable from the command line at all.
The adaptive plan is a separate structure rather than
`passes = {"Deep"}`. ProfilingEngine::Deep means "the deepest analysis
this GPU supports" - it picks SASS-or-PC plus PM and does not guarantee
the base Trace activity that kernel_launch_rate and recent_kernel_ms are
computed from. An adaptive run needs the base pinned, so the plan states
base=Trace, prepared=PM, arm=WindowOnly explicitly.
PM alone for now. On a 3090 it is the only deep engine that both works
and has been measured: prepared-but-unarmed PM showed no measurable
throughput cost against a Trace base (paired ratio x1.000, bootstrap 95%
CI [1.000, 1.002], 15 randomized blocks) at a measurable +225 MiB RSS.
PC sampling fails configuration under injection on that box and SASS
emits no records, so neither has a dormant cost anyone has measured -
and the policy should pick the deepest engine that fits an overhead
budget, not simply the deepest.
It is not. PmSamplingEngine::start() only emits a config event in WindowOnly mode; cuptiProfilerInitialize, the counter-availability lookup, Enable and SetConfig all sit inside StartPmSampling_(), which runs from onScopeStart - when the window opens. Three things followed from that and are corrected here: The CLI told users gpufl "prepares a compatible deep engine before CUDA initialises". That describes a lifecycle the code does not have. The benchmark measured PM SELECTED, not PM prepared-and-idle. The comment recording it said otherwise, and the +225 MiB RSS in particular cannot be attributed to PM preparation when no preparation happened. The window pays initialisation out of its own duration, so a late or failed init costs the front of the window. That is now written down where the next reader will see it. CaptureMode was declared and never enforced anywhere. TraceArgs is a plain struct, so anything building one directly could set both passes and deep_requested and reach runTraceCommon in a state the parser refuses - where resolvePassPlan honours passes while the deep environment is still exported. Checked again at that entry point. Not covered by a test: runTraceCommon needs a TracePlatform, and standing one up is a larger change than this guard. The help promised SASS and PC sampling behaviour that an adaptive run does not select today. It now says the selected engine is printed at startup instead of naming engines it may not use.
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.
Description
Type of Change
Testing
Windows/Linux tests and RTX 3090/5060 E2E profiling completed.
Checklist