-
Notifications
You must be signed in to change notification settings - Fork 18
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
Refactoring and unifying various thread-related code #310
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (1/2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (2/2)
Pull Request Test Coverage Report for Build 12012369000Details
💛 - Coveralls |
b127bf6
to
7de39cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the centralized naming!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
051d708
to
e797b17
Compare
Check-perf-impact results: (32e33781a20e2a3976ece8f7dc81ae2b) ❓ No new benchmark data submitted. ❓ |
e1f9642
to
2193b7d
Compare
2193b7d
to
e21469c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM!
- also deduplicate thread body boilerplate
e21469c
to
e820256
Compare
This refactoring PR performs 2 main tasks:
named_threads
enum entry as its parameterThere are no functional changes, test coverage is slightly extended.
Note that the implementation of how the try/catch block in threads is designed and what happens in response to exceptions had already slightly diverged (presumably unintentionally).
Check-perf-impact result (no new file required):
✔️ No significant performance change in the microbenchmark set. You are good to go!