Repository navigation
Conversation
* ze: host-read kernel timestamps for immediate command lists (fix O(N²)) The tracer baked a zeCommandListAppendQueryKernelTimestamps (QKT) per profiled Append into the user's immediate command list. The L0 driver's per-QKT cost is superlinear in the number of QKT ops simultaneously in flight (signaled-but-not-synced), independent of queue ordering. Workloads that signal faster than they sync grow that backlog without bound, so the tracer overhead grew O(N²) over the run (measured: kynema_sgf/AMReX Solve time 1.8s→4.3s over 10 steps under iprof; untraced flat at 0.69s). Fix: for immediate cls, do not bake a device QKT. The prologue already swaps the user's signal for our injected KERNEL_TIMESTAMP event `inj`, so the kernel/copy signals `inj` directly and it carries the real kernel timing. Append a barrier (wait=inj, sig=user_signal) to re-expose the user's event, and at drain read the timing host-side via zeEventQueryKernelTimestamp(inj). Because inj is tracer-owned it is never reused by the user, so host-read is never overwritten — no reuse detection needed. Regular cls are unchanged (they are replayed per Execute, so host-read can't capture each Execute; their in-flight QKT count is bounded by un-synced Executes, not total appends). Also replaces the O(N) _cl_any_live slot scan with an O(1) n_live counter (deletes _cl_any_live and the _ZE_FOREACH_SLOT macro). Verified: reproducer per-sync ratio flat (was 1.6/2.1/3.2 at N=32k/64k/128k → now ~1.03); correctness 58/58; benches pass; copy engine host-read timestamps valid; real app step-10 overhead 6.2×→1.7× with the O(N²) growth eliminated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ze: drop immediate-cl reset workaround (no longer needed, was residual O(N)) _imm_reset_if_drained raw-Reset the user's immediate command list every time its in-flight backlog drained to zero. Its sole purpose was to reclaim the L0 driver's per-AppendQueryKernelTimestamps storage, which accumulated on a long-lived reused immediate cl. Immediate cls no longer bake a device QKT (they host-read the injected event), so there is no such storage to reclaim, and their slot bookkeeping is already fully reclaimed by _slot_release at drain (events returned to the pool, empty non-tail chunks freed). The reset was pure cost: a full driver command-list reset on every drain-to-empty. On the real app (kynema_sgf, 50 steps) removing it cut ~0.22s off every step AND collapsed the remaining linear creep from +32% to +3.2% over the run (Solve 1.119->1.481 became 0.894->0.923; overhead vs untraced now ~1.33x and flat). The separate shadow-cl reset in _slot_drain (gated by s->sh) is kept — regular copy-only cls still bake QKT on the shadow cl. Verified: correctness 58/58, benches pass, reproducer flat, real app flat across 50 steps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ze: unify all command lists onto host-read timestamps; delete device QKT + shadow Extends the immediate-cl host-read approach to REGULAR command lists too, so ALL kernel/copy timing is captured by host-reading a tracer-owned injected KERNEL_TIMESTAMP event (zeEventQueryKernelTimestamp at drain) instead of a device-side zeCommandListAppendQueryKernelTimestamps (QKT). Deletes, now unused: - the per-(context,device) "shadow" command list (existed only to host QKT for copy queue groups, which reject QKT), - the queue-group class cache and is_compute (only chose inline-vs-shadow), - the device-written timestamp slab and per-slot offset, - _append_inline_query / _shadow_append_query / _slot_publish. Net ~ -330 lines. Scheme (one uniform path): - Append: prologue swaps the user's signal for our inj (kernel/copy signals inj); re-expose the user's signal on the user cl — in-order cls use a plain zeCommandListAppendSignalEvent (measured ~9% cheaper than a barrier: ~1500ns vs ~1640ns per op), out-of-order cls use zeCommandListAppendBarrier(wait=inj) for ordering. Then instantiate the slot. - Drain: zeEventQueryKernelTimestamp(inj); stash under the user's event. Never reset inj (a replayed regular cl's next Execute overwrites it, last-value-wins like the old slab; resetting would hang a second drain on a never-re-signaled event). inj disposed at cl Reset/Destroy (regular) or _slot_release (imm). - Execute epilogue stays one critical section (force-sync-prior + drain + re-instantiate + claim in_flight_q) so concurrent Executes observe in_flight_q atomically. INVARIANT: the tracer only ever syncs/queries/resets ITS OWN inj event; the user's event is read-only (we stash timing under it). The sole user-object sync is the Execute epilogue's zeCommandQueueSynchronize on a QUEUE (never an event) to drain a prior in-flight submission. Verified on Intel Max 1550: correctness 58/58 (serial and bats --jobs 4), all benches pass, the O(N^2) reproducer flat, reg_Event_09/10/11 and multithreaded_01/02 correct, no hang. (The hang seen in an earlier build was flakiness / an intermediate broken prologue-split, not this design.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ze: run Execute bookkeeping in the prologue; reset our injected event at drain Moves all zeCommandQueueExecuteCommandLists tracer bookkeeping (force-sync the prior in-flight instance + drain + re-instantiate slots + claim in_flight_q) from the epilogue into the PROLOGUE, as one critical section, and resets our injected event at drain instead of relying on the next replay to overwrite it. Why the prologue: a regular cl is closed once and replayed; its injected KERNEL_TIMESTAMP event is baked into the cl body and re-signaled by every Execute. Draining the previous instance AFTER the next submit (the old epilogue) races that re-signal — for back-to-back Executes with no user sync between (reg_Event_09) the previous instance's timing was clobbered by the next before we read it. Draining in the prologue reads and resets our event BEFORE this submission re-signals it, so each replay yields its own timing. The same prologue force-sync serializes a cl reused concurrently from another thread (reg_Event_multithreaded_01), so nothing needs to run after submit. Resetting our injected event at drain (rather than depending on re-signal overwriting its value) removes reliance on undocumented driver behavior. It is safe: a slot is only re-drained after being re-instantiated at a later Execute, which re-signals the event first; the final drain of a slot is never followed by another. We only ever reset OUR event, never the user's. Immediate cls reset it via the pool recycle path (_put_ze_event); only kept-across-replay regular slots need the explicit reset. Also folds in review cleanup: - rename _ze_slab_chunk/_ZE_SLAB_CHUNK_SLOTS -> _ze_slot_chunk/_ZE_CHUNK_SLOTS (no slab remains; the chunk is now purely a slot arena); - in-order cls re-expose the user signal with zeCommandListAppendSignalEvent (measured ~9% cheaper than a barrier); out-of-order cls keep the barrier; - drop the unused device/ordinal params from _on_create_command_list; use bool for is_immediate/is_in_order/live/has_kts (add <stdbool.h>); - merge the reset/destroy chunk-reclaim duplication into _cl_reclaim_chunks; - strip stale comments that described the removed QKT/shadow/slab machinery. NOT yet hardware-verified (node lost mid-session): build is clean; the prior commit is 58/58 + benches. Re-run on hardware, priority reg_Event_09 (distinct per-replay timings), multithreaded_01/02, full suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ze: reclaim emptied tail chunk at append (fix mem_persistent_cl leak) On the event-sync drain path (_on_sync -> _ZE_SYNC_EVENT -> _slot_drain), an empty slab chunk could leak on a long-lived reused immediate cl. A chunk is freed by _slot_release only when a release drives n_held to 0 AND the chunk is non-tail (the tail is guarded so new Appends keep landing on it). The one transition that leaves an empty non-tail chunk is a chunk that drained empty while it was still the tail and was later superseded by a new tail — nothing revisited it. Fix it at that single transition: when _cl_slot_append allocates a new chunk, reclaim the old tail if it is already empty (n_held==0). O(1) on a path that already allocates; every other empty non-tail chunk is still freed inline by _slot_release the moment it drains. No separate sweep pass, and any future partial-drain sync anchor gets the reclaim for free. On bench/mem_persistent_cl (reused immediate cl synced via zeEventHostSynchronize) this leaked ~582 B/round (116 KB, threshold 64 KB). Also clang-format-18 the file (fixes pre-existing comment-alignment and signature-wrapping violations). Verified on hardware: repro 116464 -> 64 B (flat), flat across NR=100..800 in every appends-per-round regime, suite 65/65. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ze: O(1) in-order predecessor via per-cl live-slot list (fix O(N^2) build) _slot_instantiate found the in-order predecessor by scanning chunks newest-to-oldest for the first still-live slot. On a long-lived in-order immediate cl whose completed Appends stay referenced cross-cl (a downstream slot in another cl waits on them, refs>0, so their chunk is not reclaimed), that scan walks all the drained-but-pinned slots ahead of the new one — O(N) per Append, O(N^2) to build the cl. Maintain a per-cl doubly-linked list of live slots in append order (cl_data->live_slots, _ze_slot.live_prev/live_next). The in-order predecessor is its tail (O(1)); slots join at instantiate and leave at drain. Bulk teardown (reset/destroy) drops the whole list by nulling the head — detached slots have owner==NULL so their later drain skips the unlink. Also bounds _slot_drain's in-order recursion depth. Measured on hardware (immediate in-order cl, completed Appends pinned by an un-synced second cl, LTTNG_UST_ZE_PROFILE direct): before: per-Append last/first-quartile ratio 2.61 at backlog 32000 (grows with N) after: ratio 1.00, flat Suite 66/66 (new bench pinned_pred_walk_scaling; verified it fails pre-fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ze: drain injected events on zeEvent/zeFenceQueryStatus, not just blocking sync Apps that observe completion by polling zeEventQueryStatus / zeFenceQueryStatus to ZE_RESULT_SUCCESS (never the blocking HostSynchronize) never triggered a drain: the tracer only recycled injected events on the blocking sync anchors. Result was an unbounded injected-event leak AND lost profiling — a slot that never drains never emits its event_profiling_results tracepoint. Hook both QueryStatus polls to the same drain anchor as their blocking counterparts (SUCCESS return carries the same completion fact). Safe: _slot_drain no-ops on already-drained slots under the state mutex, so repeated polls drain once. Drop the stale "fence QueryStatus is unsafe to hook" note. Also removes the zeEventPoolCreate/zeEventCreate prologues that forced KERNEL_TIMESTAMP|HOST_VISIBLE on pools and HOST scope on events — timing comes from our own injected event, the user's events stay as created. Covered by ze tests inorder_imm_Event_10 (event) and ooo_reg_Event_15 (fence): both fail pre-fix (0/100 and 99/100) and pass post-fix (100/100). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ze: trim tracer overhead and clean up helpers Reduce per-Append and per-slot cost in the ZE tracer, plus a pass of readability cleanup, with no change to traced behavior: - Store the cl's context at create time and read it in the Append prologue, removing the per-Append zeCommandListGetContextHandle driver call (resolved via the existing cl hash lookup). Unregistered cls now skip cleanly instead of injecting an event and rolling it back. - Move `owner` from every slot onto its chunk (all slots in a chunk share one cl): ~504 B saved per 64-slot chunk, and chunk detach becomes a single assignment instead of a per-slot loop. - Event freelists are pure LIFO stacks: use utlist LL_ (drop the unused prev pointer) instead of DL_. - Designated initializers for the property-dump structs (also fixes two previously-uninitialized ones); bool instead of int for local flags; rename _universal_record_append -> _record_append; collapse the dlopen branch; drop a duplicate include. - Comment cleanup: describe observable behavior rather than tracer internals, drop stale references (shadow cls, QKT, test names) and de-duplicate the repeated invariant restatements. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ze: batch injected events into shared pools zeEventPoolCreate is expensive, and the tracer created one pool per injected event. Allocate pools with capacity _ZE_EVENT_POOL_CAP (64) instead and hand out events by index, creating a new pool only when the current one fills. At 512 concurrent injected events this drops pool-creates from 512 to 8 (ceil 512/64); event create/recycle and every hot path are unchanged. The shared pool is owned by the per-context entry (a list of pools for teardown), not the wrapper, so the wrapper drops its event_pool field. Events still recycle through the per-context freelist; pools live until the context is destroyed. Teardown respects the L0 rule that every event be destroyed before its pool: context-destroy destroys all injected events (in-flight via the cl sweep's _ZE_DISPOSE_WRAPPER, then the freelist) before destroying any pool. Rename the ctx_dying flag to ctx_destroy and fix its now-stale comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ze: rework injected-event allocator, fix cl destroy gate, harden + document Injected-event allocator (tracer_ze_helpers.include.c): - delete the global struct-recycle pool; the per-context L0-event freelist is the only recycle. calloc/free the wrapper struct directly. - rename to intent: _get_event/_put_event (API), _internal_* (file-local; not __ which the C standard reserves), _ZE_DISPOSE_RECYCLE/_DESTROY, chunk->slab. - ZE_MUST on the teardown destroy calls; warn-once on zeEventPoolCreate failure. - remove dead NULL guards proven unreachable by every caller (_event_state_del, _event_state_get_or_add, _slot_release, _record_append !inj, _slot_drain !s); annotate the one live !ev guard that survives. - int->bool on the kts predicates. zeCommandListDestroy gate _do_profile -> _do_state(), symmetric with the create hooks: a memory-info-only config registered cls it never unregistered, leaking the registration and colliding with handle-recycled creates. Docs: top-of-file rationale + a "Timestamp profiling architecture" section in README.md (ownership / counters / state-machine / teardown, with the detach worked example). Comment editor pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix clang-format --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Thomas Applencourt <applenco@sunspot-uan-0002.head.cm.sunspot.alcf.anl.gov> Co-authored-by: Thomas Applencourt <applenco@sunspot-uan-0001.head.cm.sunspot.alcf.anl.gov>
…argonne-lcf#517) The MPI tracer shim is LD_PRELOADed as libmpi.so and dlopen()s the real libmpi. It was opened with RTLD_LOCAL, which keeps the real library's symbols out of the global scope. Any MPI symbol the shim does not itself wrap (e.g. MPIX_GPU_query_support) then fails to resolve in the traced application: ./app: symbol lookup error: ./app: undefined symbol: MPIX_GPU_query_support Switch both dlopen() calls (LTTNG_UST_MPI_LIBMPI and the libmpi.so fallback) to RTLD_GLOBAL, keeping RTLD_DEEPBIND. Our preloaded wrappers still take precedence for wrapped symbols since they are earlier in the global scope, and RTLD_DEEPBIND keeps the real lib's internal calls bound to itself rather than re-entering our wrappers. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
No description provided.