-
Notifications
You must be signed in to change notification settings - Fork 1k
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 reference implementation for parallel_phase feature #1570
Merged
+728
−46
Merged
Changes from 33 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
5e5778e
Add initial impl of Parallel Block
pavelkumbrasev 55fd835
Add test for workers_leave
pavelkumbrasev 637a74d
Fix thread_leave_manager
pavelkumbrasev eb172a7
Add parallel_block API, improve tests
isaevil 7f08af0
Improve tests for parallel_block
isaevil f6a139a
Add workers_leave::automatic, let hybric systems take advantage of pa…
isaevil 0a9faed
Fix Windows compilation
isaevil c1802a1
Add win32 exports
isaevil ecd5a9e
Add mac64 symbols
isaevil 0d32f80
Correct mask, add a clarifying comment to it
isaevil f61fc1d
Apply suggested typo fix
isaevil 17f1dac
Rename block to phase
isaevil aa7cc2a
Align with RFC, utilize my_version_and_traits, improve readability
isaevil 104606c
Fix non-preview compilation
isaevil 597136f
Change scoped_parallel_phase default parameter, fix entry points for Win
isaevil 1bc6e3a
Apply suggestions from code review
isaevil 73742f3
Apply comments, change tests
isaevil ba5b922
Correct mac64 exports
isaevil 7311cec
Improve test reporting, move thread demand into internals
isaevil 1f22bc6
Extend testing suite with scoped phases and test with this_task_arena
isaevil fd4a24f
Apply suggestions from code review
isaevil 77985c0
Simplify state transition logic
isaevil 1a6b73c
Align leave_policy variables names across code
isaevil 398d16b
Move parallel phase tests into a separate file
isaevil 4dfaf5e
Address comments
isaevil 6a951bd
Decrease the size of dummy work, don't execute tests for workerless a…
isaevil 9d507ca
Apply suggestions from code review
isaevil 515fd33
Improve test stability
isaevil 0f233bb
Use uintptr_t
isaevil 15c3ea5
Update copyright years
isaevil f041f3a
Reduce testing time
isaevil fdae151
Move RFC to the experimental stage
isaevil a178e85
Fix one time fast leave
isaevil 0f1b1a5
Fix sources format
isaevil c38eb2f
Rename entry points, simplify state machine interaction
isaevil File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes
File renamed without changes
File renamed without changes
Binary file added
BIN
+123 KB
.../experimental/parallel_phase_for_task_arena/parallel_phase_sequence_diagram.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
File renamed without changes
File renamed without changes
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is there a reason to use "register" and "unregister" in the names for the entry points? To me "registration" does not imply the start and end of the active time in the region, but might be used to indicate that a region just exists. For example, an athlete might register for a race but that doesn't mean they've started running.
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 think the main idea was to use more generic names, which do not depend on feature API. So, if API will change dramatically, entry point names would still make some sense.
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.
There might be alternative suitable generic verbs, such as "initiate", "enter", "commence", "launch", "open" for the beginning and "complete", "conclude", "close", "finish", "exit".
From the semantical point of view (i.e. speaking of a program execution phase), I would likely use "enter/exit" or "initiate/complete"; but since it's an internal entry point, I am OK with any choice made, including "register/unregister".
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.
Of the suggestions, I like "enter/exit".
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.
@akukanov @vossmjp Renamed entry points to "enter/exit".