Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 321
cuda.core: Cythonize GraphBuilder and Graph with handle-layer cleanup#2008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
0b63c00
cuda.core: convert GraphBuilder to cdef class with explicit state mac…
Andy-Jost 035b09d
cuda.core: convert Graph to cdef class with GraphExecHandle
Andy-Jost ae974af
fix(cuda.core): drop unused handle cimports flagged by cython-lint
Andy-Jost 4e71a1e
fix(cuda.core): break _stream/_device <-> graph._graph_builder import…
Andy-Jost aad9f6d
Merge branch 'main' into graph-builder-refactor
Andy-Jost 343ecca
fix(cuda.core): expose GraphBuilder._init as a Python-callable factory
Andy-Jost 9465c76
fix(cuda.core): pass non-NULL captureStatus to cuStreamGetCaptureInfo
Andy-Jost 3c6c387
Merge branch 'main' into graph-builder-refactor
Andy-Jost 7823068
test: verify Graph.close() is idempotent (Glasswing V18.1)
Andy-Jost 564a2be
cuda.core: harden GraphBuilder state machine and expand graph tests
Andy-Jost 9c02485
Merge origin/main into graph-builder-refactor
Andy-Jost 1aec3e5
fix(cuda.core): resolve merge fallout in resource handle stubs
Andy-Jost 4fccf6e
Merge origin/main into graph-builder-refactor
Andy-Jost 3177221
fix(cuda.core): pass non-NULL phGraph to cuStreamEndCapture
Andy-Jost b6c3989
Merge branch 'main' into graph-builder-refactor
rwgk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| from cuda.bindings cimport cydriver | ||
| from cuda.core._resource_handles cimport GraphExecHandle, GraphHandle, StreamHandle | ||
| from cuda.core._stream cimport Stream | ||
| cdef class GraphBuilder: | ||
| cdef: | ||
| GraphHandle _h_graph | ||
| StreamHandle _h_stream | ||
| int _kind | ||
| int _state | ||
| Stream _stream # cached to avoid reconstruction from _h_stream handle | ||
| object __weakref__ | ||
| cdef class Graph: | ||
| cdef: | ||
| GraphExecHandle _h_graph_exec | ||
| object __weakref__ | ||
| @staticmethod | ||
| cdef Graph _init(cydriver.CUgraphExec graph_exec) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,6 +8,8 @@ from cuda.core._stream import Stream | ||
| from cuda.core._utils.cuda_utils import driver | ||
| from cuda.core.graph._graph_definition import GraphCondition, GraphDefinition | ||
| _BuilderKind = int | ||
| _CaptureState = int | ||
| @dataclass | ||
| class GraphDebugPrintOptions: | ||
| @@ -106,23 +108,19 @@ class GraphBuilder: | ||
| """ | ||
| class _MembersNeededForFinalize: | ||
| __slots__ = ('conditional_graph', 'graph', 'is_join_required', 'is_stream_owner', 'stream') | ||
| def __init__(self, graph_builder_obj: GraphBuilder, stream_obj: Stream | None, is_stream_owner: bool, conditional_graph, is_join_required: bool) -> None: | ||
| ... | ||
| def close(self) -> None: | ||
| ... | ||
| __slots__ = ('__weakref__', '_building_ended', '_mnff') | ||
| def __init__(self): | ||
| ... | ||
| def __init__(self) -> None: | ||
| def __dealloc__(self): | ||
| ... | ||
| @classmethod | ||
| def _init(cls, stream: Stream | None, is_stream_owner: bool, conditional_graph: object=None, is_join_required: bool=False) -> GraphBuilder: | ||
| @staticmethod | ||
| def _init(stream: Stream): | ||
| ... | ||
| def close(self): | ||
| """Destroy the graph builder.""" | ||
| @property | ||
| def stream(self) -> Stream: | ||
| """Returns the stream associated with the graph builder.""" | ||
| @@ -155,7 +153,7 @@ class GraphBuilder: | ||
| def end_building(self) -> GraphBuilder: | ||
| """Ends the building process.""" | ||
| def complete(self, options: GraphCompleteOptions | None=None) -> 'Graph': | ||
| def complete(self, options: GraphCompleteOptions | None=None) -> Graph: | ||
| """Completes the graph builder and returns the built :obj:`~graph.Graph` object. | ||
| Parameters | ||
| @@ -245,9 +243,6 @@ class GraphBuilder: | ||
| A condition variable for controlling conditional execution. | ||
| """ | ||
| def _cond_with_params(self, node_params: object) -> tuple[GraphBuilder, ...]: | ||
| ... | ||
| def if_then(self, condition: GraphCondition) -> GraphBuilder: | ||
| """Adds an if condition branch and returns a new graph builder for it. | ||
| @@ -335,15 +330,7 @@ class GraphBuilder: | ||
| """ | ||
| def close(self) -> None: | ||
leofang marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| """Destroy the graph builder. | ||
| Closes the associated stream if we own it. Borrowed stream | ||
| object will instead have their references released. | ||
| """ | ||
| def embed(self, child: GraphBuilder) -> None: | ||
| def embed(self, child: GraphBuilder): | ||
| """Embed a previously-built :obj:`~graph.GraphBuilder` as a child node. | ||
| Parameters | ||
| @@ -392,21 +379,7 @@ class Graph: | ||
| """ | ||
| class _MembersNeededForFinalize: | ||
| __slots__ = 'graph' | ||
| def __init__(self, graph_obj: Graph, graph: driver.CUgraphExec) -> None: | ||
| ... | ||
| def close(self) -> None: | ||
| ... | ||
| __slots__ = ('__weakref__', '_mnff') | ||
| def __init__(self) -> None: | ||
| ... | ||
| @classmethod | ||
| def _init(cls, graph: driver.CUgraphExec) -> Graph: | ||
| def __init__(self): | ||
| ... | ||
| def close(self) -> None: | ||
| @@ -457,5 +430,5 @@ class Graph: | ||
| """ | ||
| __all__ = ['Graph', 'GraphBuilder', 'GraphCompleteOptions', 'GraphDebugPrintOptions'] | ||
| def _instantiate_graph(h_graph, options: GraphCompleteOptions | None=None) -> 'Graph': | ||
| def _instantiate_graph(h_graph, options: GraphCompleteOptions | None=None) -> Graph: | ||
| ... | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design question, not a blocker: this is the only one of the graph-family boxes that holds no extra state —
GraphBoxtracksh_parent,GraphNodeBoxtracksh_graph, butGraphExecBoxhas just the rawCUgraphExec. That's semantically correct today: aftercuGraphInstantiateWithParams, the exec is independent of its sourceCUgraph, andcuGraphExecUpdatetakes the new source at call time, so nothing needs to be held.But the PR description calls this "groundwork for step 3 of #1330 (graph updates)". If that work ends up wanting to remember which
GraphBuilder/CUgraphan exec was last updated from (for diagnostics, error messages, or to extend the source's lifetime through the exec), this Box is the natural place to grow aGraphHandle h_source_graph;. Worth either a one-line// TODO:noting that, or a confirmation that no such reference is planned and removing the asymmetry from any reviewer's mental model.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed: no source-graph ref on
GraphExecBoxis planned. The exec is independent post-instantiate andGraph.update()supplies the source at call time. Clarification: step 3 lays the foundation for source graph updates; exec graph updates are step 5.