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

add debug_async_scope #536

Merged

Conversation

janondrusek
Copy link
Contributor

v1::debug_async_scope and v2::debug_async_scope wrap the
corresponding async_scopes and can be used as a direct replacement
for debugging purposes. They keep track of nested operation states, allowing to identify the 'stuck' operations especially in case of a deadlock.

@janondrusek janondrusek requested a review from ispeters May 31, 2023 21:22
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 31, 2023
Copy link
Contributor

@ispeters ispeters left a comment

Choose a reason for hiding this comment

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

Basically LGTM but you left several TODOs behind.

include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Show resolved Hide resolved
include/unifex/v1/debug_async_scope.hpp Show resolved Hide resolved
include/unifex/v2/debug_async_scope.hpp Outdated Show resolved Hide resolved
test/debug_async_scope_test.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@ispeters ispeters left a comment

Choose a reason for hiding this comment

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

Basically LGTM but you left several TODOs behind.

@janondrusek janondrusek force-pushed the debug-async-scope branch 2 times, most recently from fa2eb32 to 7dd94c8 Compare June 1, 2023 00:35
Copy link
Contributor

@ispeters ispeters left a comment

Choose a reason for hiding this comment

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

Some nits and some value category errors, but otherwise good.

include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/detail/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/v1/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/v2/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/v2/debug_async_scope.hpp Outdated Show resolved Hide resolved
include/unifex/v2/debug_async_scope.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@ispeters ispeters left a comment

Choose a reason for hiding this comment

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

Some nits and some value category errors, but otherwise good.

`v1::debug_async_scope` and `v2::debug_async_scope` wrap the
  corresponding `async_scope`s and can be used as a direct replacement
for debugging purposes. They keep track of nested operation states,
allowing to identify the 'stuck' operations especially in case of a deadlock.
@janondrusek janondrusek merged commit 4ed2411 into facebookexperimental:main Jun 1, 2023
@janondrusek janondrusek deleted the debug-async-scope branch June 1, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants