Skip to content

Disable collecting timing information for cuda and hip events #356

Merged
krasznaa merged 5 commits intoacts-project:mainfrom
m-fila:eventDisableTiming
Apr 8, 2026
Merged

Disable collecting timing information for cuda and hip events #356
krasznaa merged 5 commits intoacts-project:mainfrom
m-fila:eventDisableTiming

Conversation

@m-fila
Copy link
Copy Markdown
Contributor

@m-fila m-fila commented Mar 25, 2026

The CUDA and HIP events in vecmem are used only for synchronization, not for timing, and are never leaked to the outside. Both CUDA and HIP documentation mentions that if not needed collecting timing information can be disabled to possibly give better performance

Having said that, I don't see noticeable performance improvement for traccc main branch with CUDA 12.8 on neither Nvidia RTX 3060 nor L40S.

Copy link
Copy Markdown
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Thanks for opening the PR! 😄

I'm really not sure about the best flags to use. I guess in the end we may want to allow the async_copy constructors to receive an unsinged int flags argument, which the user could set themselves if they wanted to. 🤔 Until we do that, I guess using this PR's flag setup would be reasonable.

Just to please my personal preferences, please just change the API of the internal create_event() functions. Then we could get this in.

Comment thread cuda/src/utils/event_pool.cpp
Comment thread cuda/src/utils/event_pool.cpp Outdated
@m-fila
Copy link
Copy Markdown
Contributor Author

m-fila commented Mar 25, 2026

I'm really not sure about the best flags to use. I guess in the end we may want to allow the async_copy constructors to receive an unsinged int flags argument, which the user could set themselves if they wanted to. 🤔 Until we do that, I guess using this PR's flag setup would be reasonable.

I think we could be a little opinionated here since the events are not exposed outside the vecmem so we know how they are used - for instance we don't need timing, don't need sharing events across process, don't need disabling system fences etc. Out of the flags available today it leaves only customizing whether synchronization spins or sleeps (cudaEventBlockingSync).
But here I just wanted to disable timing since it's clear that it isn't used

Copy link
Copy Markdown
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Let's get this finally in. 😄

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 8, 2026

@krasznaa krasznaa merged commit b7321b6 into acts-project:main Apr 8, 2026
29 checks passed
@m-fila m-fila deleted the eventDisableTiming branch April 9, 2026 06:47
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.

2 participants