Skip to content

Conversation

@mzient
Copy link
Contributor

@mzient mzient commented Oct 21, 2025

Category:

New feature (non-breaking change which adds functionality)

Description:

Prior to this change, the CUDAStreamPool used a per-device stream stack. This caused the most recently returned stream to be used, increasing the likelihood of getting a stream with pending work scheduled. This PR adds another "busy" list to which the streams are initially returned if they're not ready. Then, if the ready list is empty, the busy list will be traversed in search of ready streams.

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

  • Existing tests apply
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [36983125]: BUILD STARTED

@@ -1,4 +1,4 @@
// Copyright (c) 2021-2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// Copyright (c) 2021-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any additional testing needed or the one which exists is sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, theoretically I could add a test that schedules some work on a stream, returns it, picks a new stream and verifies that it's not busy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@JanuszL JanuszL self-assigned this Oct 21, 2025
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [36983125]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [36983125]: BUILD PASSED

@mzient mzient force-pushed the stream_pool_with_busy_list branch from d5c23c4 to 5f97acc Compare October 22, 2025 09:33
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [37043200]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [37046256]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [37043200]: BUILD PASSED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [37046256]: BUILD PASSED

mzient and others added 3 commits October 22, 2025 18:27
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
@mzient mzient force-pushed the stream_pool_with_busy_list branch from a90c08d to 52179fe Compare October 22, 2025 16:27
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [37060756]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [37060756]: BUILD PASSED

@mzient mzient merged commit 52a00c9 into NVIDIA:main Oct 22, 2025
6 checks passed
mdabek-nvidia pushed a commit to mdabek-nvidia/DALI that referenced this pull request Nov 27, 2025
… pool. (NVIDIA#6072)

* Add a busy list to CUDAStreamPool. Don't return busy streams from thread pool.
* Extend stream pool tests.

---------
Signed-off-by: Michal Zientkiewicz <[email protected]>
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.

4 participants