Skip to content
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

#0: Don't return shared ptrs of global sems/cbs, and directly return the object instead #16354

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

tt-aho
Copy link
Contributor

@tt-aho tt-aho commented Dec 30, 2024

global sems/cbs are natively thread safe now, so user can decide whether to use shared ptrs or not

Ticket

Link to Github Issue

Problem description

Global semaphore/cb creation do not need to return shared pointers, and user can decide whether to create shared pointers or not.

What's changed

Change global semaphore/circular buffer creation functions to return the object instead of a shared pointer and update use cases. Enable copy/move ctors for these objects.

Checklist

@tt-aho tt-aho force-pushed the aho/global-updates branch 4 times, most recently from 995de8b to 451a98a Compare January 2, 2025 17:11
…the object instead

global sems/cbs are natively thread safe now, so user can decide whether to use shared ptrs or not
@SeanNijjar
Copy link
Contributor

FYI @xuncaiTT

@tt-aho - Does this mean the main reason(s) for migrating GlobalSemaphore creation to outside of CCL op invocations now are

  1. Explicit resource control given to user
  2. Avoid unnecessary creation of global semaphore on rerun of op (when the op would otherwise is program cache)?

@tt-aho
Copy link
Contributor Author

tt-aho commented Jan 2, 2025

FYI @xuncaiTT

@tt-aho - Does this mean the main reason(s) for migrating GlobalSemaphore creation to outside of CCL op invocations now are

  1. Explicit resource control given to user
  2. Avoid unnecessary creation of global semaphore on rerun of op (when the op would otherwise is program cache)?

This pr doesn't change or fix any behaviour for global sems and ccls. This just enables users to pass/copy the object around similar to tensors, instead of forcing it to be wrapped in a shared ptr, so the original reasons for migrating the sem creation outside still holds. Ccls can still create/return shared ptrs if needed if we want to maintain that behaviour.

@tt-aho tt-aho merged commit a9c2b0e into main Jan 3, 2025
222 checks passed
@tt-aho tt-aho deleted the aho/global-updates branch January 3, 2025 06:23
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