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

make CollectTrace for profiling by iteration async #966

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

staugust
Copy link
Contributor

This fix issue #953. Makes libkineto::api().client()->stop() and stopTraceInternal run in profilerThread_ so that, the training process will not be blocked.

libkineto/src/CuptiActivityProfiler.h Outdated Show resolved Hide resolved
libkineto/src/CuptiActivityProfiler.cpp Outdated Show resolved Hide resolved
libkineto/src/CuptiActivityProfiler.cpp Show resolved Hide resolved
libkineto/src/CuptiActivityProfiler.cpp Outdated Show resolved Hide resolved
if (!stopByIterThread) {
std::lock_guard<std::mutex> guard(mutex_);
if (!stopByIterThread) {
stopByIterThread = new std::thread([collection_done, this, now](){
Copy link
Contributor

Choose a reason for hiding this comment

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

std_make_unique

libkineto/src/CuptiActivityProfiler.cpp Outdated Show resolved Hide resolved
std::lock_guard<std::mutex> guard(mutex_);
if (stopByIterThread) {
stopByIterThread->join();
delete stopByIterThread;
Copy link
Contributor

Choose a reason for hiding this comment

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

with unique ptr this woudl be stopByIterThread.reset()

libkineto/src/CuptiActivityProfiler.cpp Outdated Show resolved Hide resolved
@staugust
Copy link
Contributor Author

staugust commented Aug 2, 2024

Thank you for those helpful tips, I'll improve this pr as comments advised.

@staugust
Copy link
Contributor Author

@briancoutinho It seems this pr is ignored after last commit. I've rebased master branch, please take some time to re-review this pr, thank you.

@briancoutinho
Copy link
Contributor

briancoutinho commented Aug 31, 2024 via email

@staugust
Copy link
Contributor Author

staugust commented Sep 2, 2024

@briancoutinho it's ok. @sraikund16 would you like to review this pr?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants