Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ public:
{
/* This will lookup in the cache (if any) and update an existing entry, or
* instantiate a graph if none is found. */
auto query_result = async_resources().cached_graphs_query(nnodes, nedges, *g);
auto query_result = async_resources().cached_graphs_query(nnodes, nedges, *g, state.submitted_stream);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one open design decision after 739706b: this path binds cache entries to state.submitted_stream, i.e. the USER's stream, whose lifetime STF does not control. With id binding a recycled handle can no longer false-match, but entries bound to destroyed user streams become permanent zombies (never matched, never reclaimed — reclaim cannot prove the dead stream's last launch drained). Options: (a) accept bounded zombie growth (one entry per destroyed-stream x topology, ~10KB/node estimate) and document it; (b) don't insert into the cache on this path — user-stream submits instantiate uncached, losing reuse but keeping the cache zombie-free; (c) have graph_ctx wrap user streams into pool-owned proxies at creation so the ownership invariant covers everything. The stackable path (pick_stream()) is pool-owned and safe by construction either way. My lean is (a) now with a comment, (c) as the eventual clean state.

state.exec_graph = query_result.first;

hit = query_result.second; // indicate if this was a hit or miss in the cache
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,13 @@ public:
// The graph is only used during the call (to update or instantiate); it is never stored, so the
// caller only needs to keep it valid for the duration of the call.
::cuda::std::pair<::std::shared_ptr<cudaGraphExec_t>, bool>
cached_graphs_query(size_t nnodes, size_t nedges, cudaGraph_t g)
cached_graphs_query(size_t nnodes, size_t nedges, cudaGraph_t g, cudaStream_t stream)
{
_CCCL_ASSERT(pimpl, "async_resources_handle is not initialized");
return pimpl->cached_graphs.query(nnodes, nedges, g);
return pimpl->cached_graphs.query(nnodes, nedges, g, stream);
}

::cuda::std::pair<::std::shared_ptr<cudaGraphExec_t>, bool> cached_graphs_query(cudaGraph_t g)
::cuda::std::pair<::std::shared_ptr<cudaGraphExec_t>, bool> cached_graphs_query(cudaGraph_t g, cudaStream_t stream)
{
const size_t nnodes = cuda_try<cudaGraphGetNodes>(g, nullptr);
#if _CCCL_CTK_AT_LEAST(13, 0)
Expand All @@ -242,7 +242,7 @@ public:
#endif // _CCCL_CTK_AT_LEAST(13, 0)

_CCCL_ASSERT(pimpl, "async_resources_handle is not initialized");
return cached_graphs_query(nnodes, nedges, g);
return cached_graphs_query(nnodes, nedges, g, stream);
}

#if _CCCL_CTK_AT_LEAST(12, 4)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#include <cuda/experimental/__stf/utility/pretty_print.cuh>
#include <cuda/experimental/__stf/utility/source_location.cuh>

#include <queue> // for ::std::priority_queue
#include <mutex>
#include <unordered_map>

namespace cuda::experimental::stf
Expand Down Expand Up @@ -119,12 +119,18 @@ public:
// One entry of the cache
struct entry
{
entry(executable_graph_cache* cache, ::std::shared_ptr<cudaGraphExec_t> exec_g_, size_t footprint)
entry(executable_graph_cache* cache,
::std::shared_ptr<cudaGraphExec_t> exec_g_,
cudaStream_t stream_,
unsigned long long stream_id_,
size_t footprint)
: cache(cache)
, exec_g(mv(exec_g_))
, stream(stream_)
, stream_id(stream_id_)
, footprint(footprint)
{
last_use = cache->index;
last_use = cache->index++;
}

// Update the last_use field to mark that this entry was used recently
Expand All @@ -135,6 +141,14 @@ public:

executable_graph_cache* cache;
::std::shared_ptr<cudaGraphExec_t> exec_g;
// The binding identity is the driver-assigned stream id, which is unique
// for the lifetime of the process: a cudaStream_t handle value can be
// recycled after cudaStreamDestroy, so comparing handles could falsely
// match an entry bound to a dead stream against an unrelated new one.
// The raw handle is kept only to probe idleness, which is meaningful
// only while the bound stream is alive (see query_stream_state).
cudaStream_t stream;
unsigned long long stream_id;
size_t last_use;
size_t footprint;
};
Expand All @@ -156,15 +170,32 @@ public:
// Check if there is a matching entry (and update it if necessary)
// the returned bool indicate is this is a cache hit (true = cache hit, false = cache miss)
// The graph g is only used during this call (for update or instantiate); it is never stored.
::cuda::std::pair<::std::shared_ptr<cudaGraphExec_t>, bool> query(size_t nnodes, size_t nedges, cudaGraph_t g)
::cuda::std::pair<::std::shared_ptr<cudaGraphExec_t>, bool>
query(size_t nnodes, size_t nedges, cudaGraph_t g, cudaStream_t stream)
{
::std::lock_guard<::std::mutex> guard(mutex);

int dev_id = cuda_try<cudaGetDevice>();
_CCCL_ASSERT(dev_id < int(cached_graphs.size()), "invalid device id value");

const unsigned long long stream_id = stream_unique_id(stream);

auto range = cached_graphs[dev_id].equal_range({nnodes, nedges});
for (auto it = range.first; it != range.second; ++it)
{
auto& e = it->second;
// Executable graphs are only reused on the stream to which the cache
// entry is bound. In addition to preventing CUDA from serializing
// concurrent launches of one executable on different streams, this
// gives us an explicit completion check before the host-side update.
// The caller's stream is alive by definition, so probing it is safe;
// a caller stream in capture reads as busy (a query would invalidate
// the capture), falling through to a fresh instantiation.
if (e.stream_id != stream_id || query_stream_state(stream) != stream_state::idle)
{
continue;
}

if (reserved::try_updating_executable_graph(*e.exec_g, g))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for a comment rather than a change: the idle-check-then-update sequence is protected by the cache mutex against other QUERIES, but nothing stops a second host thread from LAUNCHING onto this same stream between the check and the update. The design is sound under the invariant "one support stream is submitted to by one thread at a time", which holds today for pool streams and single-ctx user streams, but it is implicit. One sentence here would make it an invariant instead of luck.

Related, pre-existing (the function itself is outside this diff so noting it here at its call site): try_updating_executable_graph's blind cudaGetLastError() also swallows any unrelated earlier pending async error, reporting it as "update failed" (a silent miss) instead of surfacing it. Cheap hardening: check the return value of cudaGraphExecUpdate itself and only clear-and-classify when that call is what failed.

{
// update the last use index for the LRU algorithm
Expand All @@ -191,55 +222,93 @@ public:
// If we maintain a cache, store the executable graph
if (cache_size_limit != 0)
{
cached_graphs[dev_id].insert({::std::make_pair(nnodes, nedges), entry(this, exec_g, footprint)});
cached_graphs[dev_id].insert(
{::std::make_pair(nnodes, nedges), entry(this, exec_g, stream, stream_id, footprint)});
total_cache_footprint[dev_id] += footprint;
}

return ::cuda::std::make_pair(exec_g, false);
}

private:
void reclaim(int dev_id, size_t to_reclaim)
// The driver-assigned stream id: unique for the process lifetime, unlike
// the handle value (see entry::stream_id).
static unsigned long long stream_unique_id(cudaStream_t stream)
{
size_t reclaimed = 0;

// Use a priority queue (min-heap) to track least recently used entries
using key_type = ::std::pair<size_t, size_t>;

auto& device_cache = cached_graphs[dev_id];

auto cmp = [&device_cache](const key_type& key_a, const key_type& key_b) {
auto iter_a = device_cache.find(key_a);
auto iter_b = device_cache.find(key_b);

// Directly compare last_use timestamps
return iter_a->second.last_use > iter_b->second.last_use;
};
unsigned long long id = 0;
cuda_safe_call(cudaStreamGetId(stream, &id));
return id;
}

// Priority queue storing keys, ordered by least recently used
::std::priority_queue<key_type, ::std::vector<key_type>, decltype(cmp)> lru_queue(cmp);
enum class stream_state
{
idle,
busy,
unavailable
};

// Populate queue with keys from the cache
for (const auto& kv : device_cache)
// Probe a stream without ever throwing and without touching a capture:
// cudaStreamQuery on a capturing stream would invalidate that capture (a
// cross-thread hazard when reclaim probes another context's stream), so
// capture status is checked first with the capture-legal API. Errors from
// either call (e.g. a destroyed handle for an entry whose bound stream the
// cache does not own) read as `unavailable`: such an entry is neither
// reusable nor provably safe to destroy.
static stream_state query_stream_state(cudaStream_t stream)
{
cudaStreamCaptureStatus capture = cudaStreamCaptureStatusNone;
if (cudaStreamIsCapturing(stream, &capture) != cudaSuccess)
{
lru_queue.push(kv.first);
cudaGetLastError();
return stream_state::unavailable;
}

// Reclaim least recently used entries
while (!lru_queue.empty() && reclaimed < to_reclaim)
if (capture != cudaStreamCaptureStatusNone)
{
return stream_state::busy;
}
const cudaError_t status = cudaStreamQuery(stream);
if (status == cudaSuccess)
{
key_type key = lru_queue.top();
lru_queue.pop();
return stream_state::idle;
}
cudaGetLastError();
return (status == cudaErrorNotReady) ? stream_state::busy : stream_state::unavailable;
}

// Find the entry before erasing
auto it = device_cache.find(key);
if (it != device_cache.end())
void reclaim(int dev_id, size_t to_reclaim)
{
size_t reclaimed = 0;
auto& device_cache = cached_graphs[dev_id];

// Reclaim the least-recently-used idle entries. cudaGraphExecDestroy must
// not race an in-flight launch, so a busy entry remains cached even if
// that temporarily leaves the cache above its configured size. An
// `unavailable` entry (bound stream destroyed) is skipped too: its final
// launch may still be draining, so destroying the executable is not
// provably safe, and the entry stays as an unreclaimable zombie. This is
// benign when cache-bound streams outlive the cache (the pool streams
// handed out by async_resources_handle do); binding entries to streams
// with independent lifetimes is what makes zombies possible at all.
while (reclaimed < to_reclaim)
{
auto victim = device_cache.end();
for (auto it = device_cache.begin(); it != device_cache.end(); ++it)
{
reclaimed += it->second.footprint;
total_cache_footprint[dev_id] -= it->second.footprint;
if (query_stream_state(it->second.stream) == stream_state::idle

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

important: Do not probe entry::stream after its owner can call cudaStreamDestroy. A cached entry can retain this raw handle after destruction, and query_stream_state then calls cudaStreamIsCapturing through it during reclamation. CUDA defines use of a stream handle after cudaStreamDestroy as undefined, so the intended unavailable path cannot safely prevent a crash or other invalid behavior. Track launch completion with cache-owned events, or enforce that every bound stream outlives the cache. (docs.nvidia.com)

As per path instructions, focus on lifetime/resource ownership.

Source: Path instructions

&& (victim == device_cache.end() || it->second.last_use < victim->second.last_use))
{
victim = it;
}
}

device_cache.erase(it);
if (victim == device_cache.end())
{
break;
}

reclaimed += victim->second.footprint;
total_cache_footprint[dev_id] -= victim->second.footprint;
device_cache.erase(victim);
}
}

Expand All @@ -253,5 +322,10 @@ private:
::std::vector<size_t> total_cache_footprint;

size_t cache_size_limit;

// A handle may be shared by multiple host threads. Serialize cache lookup,
// update, insertion, and reclaim so one executable cannot be updated by two
// queries concurrently.
::std::mutex mutex;
};
} // namespace cuda::experimental::stf
Original file line number Diff line number Diff line change
Expand Up @@ -685,8 +685,9 @@ public:
cuda_try(cudaGraphGetEdges(graph, nullptr, nullptr, &nedges));
#endif

auto [cached_exec, cache_hit] = ctx.async_resources().cached_graphs_query(nnodes, nedges, graph);
exec_graph_ = mv(cached_exec);
auto [cached_exec,
cache_hit] = ctx.async_resources().cached_graphs_query(nnodes, nedges, graph, support_stream);
exec_graph_ = mv(cached_exec);

auto* cache_stat = ctx.graph_get_cache_stat();
if (cache_stat)
Expand Down
1 change: 1 addition & 0 deletions cudax/test/stf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ set(
reductions/sum_multiple_places_no_refvalue.cu
slice/pinning.cu
stackable/composite_conditional.cu
stackable/executable_graph_cache_streams.cu
stackable/graph_scope_test.cu
stencil/stencil-1D.cu
stress/empty_tasks.cu
Expand Down
51 changes: 28 additions & 23 deletions cudax/test/stf/graph/get_cache_stats.cu
Original file line number Diff line number Diff line change
Expand Up @@ -19,33 +19,38 @@ using namespace cuda::experimental::stf;

int main()
{
async_resources_handle handle;
for (size_t i = 0; i < 10; i++)
cudaStream_t stream = cuda_try<cudaStreamCreate>();
{
graph_ctx ctx(handle);
auto lA = ctx.logical_data(shape_of<slice<size_t>>(64));
ctx.launch(lA.write())->*[] _CCCL_DEVICE(auto t, slice<size_t> A) {
for (auto i : t.apply_partition(shape(A)))
async_resources_handle handle;
for (size_t i = 0; i < 10; i++)
{
graph_ctx ctx(stream, handle);
auto lA = ctx.logical_data(shape_of<slice<size_t>>(64));
ctx.launch(lA.write())->*[] _CCCL_DEVICE(auto t, slice<size_t> A) {
for (auto i : t.apply_partition(shape(A)))
{
A(i) = 2 * i;
}
};
ctx.finalize();
cuda_try(cudaStreamSynchronize(stream));

// Query statistics about the graph context : the first iteration needs to
// instantiate the graph, then we will reuse graphs saved in the handle.
auto* st = ctx.graph_get_cache_stat();
if (i == 0)
{
EXPECT(st->instantiate_cnt == 1);
EXPECT(st->update_cnt == 0);
}
else
{
A(i) = 2 * i;
EXPECT(st->instantiate_cnt == 0);
EXPECT(st->update_cnt == 1);
}
};
ctx.finalize();

// Query statistics about the graph context : the first iteration needs to
// instantiate the graph, then we will reuse graphs saved in the handle.
auto* st = ctx.graph_get_cache_stat();
if (i == 0)
{
EXPECT(st->instantiate_cnt == 1);
EXPECT(st->update_cnt == 0);
}
else
{
EXPECT(st->instantiate_cnt == 0);
EXPECT(st->update_cnt == 1);
// fprintf(stderr, "nnodes %ld nedges %ld\n", st->nnodes, st->nedges);
}

// fprintf(stderr, "nnodes %ld nedges %ld\n", st->nnodes, st->nedges);
}
cuda_try(cudaStreamDestroy(stream));
}
Loading