Skip to content

[SYCL] Synchronize event lifetime with graph during recording - #23433

Closed
kweronsx wants to merge 1 commit into
intel:syclfrom
kweronsx:detached_event_alloc_for_graph
Closed

kweronsx wants to merge 1 commit into
intel:syclfrom
kweronsx:detached_event_alloc_for_graph

Conversation

@kweronsx

@kweronsx kweronsx commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

If an event is destroyed it returns to the pool. The problem with this, is that a recorded graph's lifetime may exceed the lifetime of the events of its commands, which could lead to a situation where an event is reported to be used at the same time by the graph and some other function.

This change aims to prevent event recycling in the case of graph recording by making the graph store the events it uses.

@kweronsx
kweronsx force-pushed the detached_event_alloc_for_graph branch 3 times, most recently from ff553df to 4b4ef2f Compare October 13, 2026 13:55
@kweronsx kweronsx changed the title Use detached event allocation during graph recording Synchronize event lifetime with graph during recording Oct 14, 2026
Comment on lines +26 to +27
// Must not take the queue lock: it is called while that lock may be held.
virtual bool isGraphCaptureActiveNoLock() = 0;

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.

may be held, or is held? Usually, the name we use for such functions is ...Unlocked.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It should be 'is held'; I'll also update the name accordingly.

This change allocates detached events during graph recording,
but it also synchronizes the lifetime of these events by
having the graph store them. Their non-owning references
are returned to the caller of queue::submit.
@kweronsx
kweronsx force-pushed the detached_event_alloc_for_graph branch from 4b4ef2f to c1c7f56 Compare October 15, 2026 13:25
@kweronsx kweronsx changed the title Synchronize event lifetime with graph during recording [SYCL] Synchronize event lifetime with graph during recording Oct 16, 2026
@kswiecicki

Copy link
Copy Markdown
Contributor

I've talked with @mmichel11, about the graph event lifetime problem. For the time being, the optimal solution would be to cache the event handles associated with a graph in the ur_graph_handle_t structure, and defer their release until the graph structure is destroyed.

After that, we could think about redistributing such cached events for the same graph (a free-list stored in graph struct) as a next optimization step.

@kweronsx

Copy link
Copy Markdown
Contributor Author

This change was discontinued in favor of: #23516

@kweronsx kweronsx closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants