Improve parallelism by solving most difficult deals first #216
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,8 @@ | |
|
|
||
| #include <algorithm> | ||
| #include <atomic> | ||
| #include <iterator> | ||
| #include <numeric> | ||
| #include <thread> | ||
| #include <vector> | ||
|
|
||
|
|
@@ -31,23 +33,57 @@ auto resolve_worker_count( | |
| } | ||
|
|
||
|
|
||
| static auto is_permutation_of_range( | ||
| const std::vector<int>& order, | ||
| const int count) -> bool | ||
| { | ||
|
Collaborator
There was a problem hiding this comment. We have Standard library functions are at least expected to be faster and reduce the amount of code we need to write.
Collaborator
Author
There was a problem hiding this comment. Or not… Copilot reports that the change is less efficient: is_permutation_of_range() currently re-sorts the order vector and builds an intermediate common vector via set_intersection. This is O(n log n) plus extra allocations and will run on every parallel_all_boards_n() invocation that supplies an order (e.g. batch calc), adding overhead in a performance-sensitive path. You can validate a permutation of [0, count) in O(n) without sorting by tracking seen indices and checking bounds/duplicates. Thoughts, @zzcgumn?
Collaborator
Author
There was a problem hiding this comment. I've had another look. I agree with Copilot - I prefer the original implementation:
Contributor
There was a problem hiding this comment. set_difference makes sense if you need to know what the difference is. The use case here doesn't need to prove as hard a problem. |
||
| std::vector<int> sorted(order); | ||
| std::sort(sorted.begin(), sorted.end()); | ||
|
|
||
| std::vector<int> expected(static_cast<unsigned>(count)); | ||
| std::iota(expected.begin(), expected.end(), 0); | ||
|
|
||
| std::vector<int> common; | ||
| common.reserve(static_cast<unsigned>(count)); | ||
| std::set_intersection( | ||
| sorted.begin(), sorted.end(), | ||
| expected.begin(), expected.end(), | ||
| std::back_inserter(common)); | ||
|
|
||
| return static_cast<int>(common.size()) == count; | ||
| } | ||
|
|
||
|
|
||
| auto parallel_all_boards_n( | ||
| const int count, | ||
| const int worker_cap, | ||
| const std::function<int(int worker_id, int bno)>& process_board, | ||
| const std::vector<int>* order) -> int | ||
| { | ||
| if (count <= 0) | ||
| { | ||
| return RETURN_NO_FAULT; | ||
| } | ||
|
|
||
| // Map a dispatch slot to the board number to process. With an order, hand out | ||
| // boards in that sequence (e.g. hardest first); otherwise in index order. The | ||
| // order is only honored when it is a valid permutation of [0, count); a | ||
| // malformed order falls back to index order to avoid invalid board indices. | ||
| const bool use_order = | ||
| (order != nullptr && | ||
| static_cast<int>(order->size()) == count && | ||
| is_permutation_of_range(*order, count)); | ||
| auto board_of = [&](const int slot) -> int { | ||
| return use_order ? (*order)[static_cast<unsigned>(slot)] : slot; | ||
| }; | ||
|
|
||
| const int workers = resolve_worker_count(worker_cap, count); | ||
|
|
||
| if (workers == 1) | ||
| { | ||
| for (int slot = 0; slot < count; ++slot) | ||
| { | ||
| const int rc = process_board(0, board_of(slot)); | ||
| if (rc != RETURN_NO_FAULT) | ||
| { | ||
| return rc; | ||
|
|
@@ -62,11 +98,12 @@ auto parallel_all_boards_n( | |
| auto worker = [&](const int worker_id) { | ||
| for (;;) | ||
| { | ||
| const int slot = next.fetch_add(1, std::memory_order_relaxed); | ||
| if (slot >= count || first_error.load(std::memory_order_relaxed) != RETURN_NO_FAULT) | ||
| { | ||
| break; | ||
| } | ||
| const int bno = board_of(slot); | ||
|
|
||
| const int rc = process_board(worker_id, bno); | ||
| if (rc != RETURN_NO_FAULT) | ||
|
|
||
This file contains hidden or 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 hidden or 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 hidden or 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
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.
Might as well fix the indenting problem at the same time.