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.
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
[oneDPL][ranges] support size limit for output for merge algorithm #1942
[oneDPL][ranges] support size limit for output for merge algorithm #1942
Changes from all commits
dc365d6
40fda0a
9fe0f5d
faa05ec
d625259
5c44ba6
094506e
2df51e3
9933db8
aa8a630
c1e4c6c
f95e2a8
606f913
ad286c9
50d1fdf
13b8b32
2ecee1b
575ce06
aaf3d6f
ee96772
8760dd2
f8793b3
501c947
743f7a4
6087840
38f2fe0
bee6176
5af750c
2a56a21
6915343
4002984
b6eae0e
f2e117b
3583a99
d0dcc89
b3a6db0
94322bb
c793245
a5ac234
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is funny :) @MikeDvorskiy you explained to me that using indexes as the parameters of this function is to avoid computing the indexes twice. But in the serial implementation you do not really use indexes but switch back to the iterators :) which only confirms my impression that
___merge_path_out_lim
should use iterators as its parameters.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 told that I was going to avoid twice computation for new output size
const _Index3 __n_out = std::min<_Index3>(__n_1 + __n_2, std::ranges::size(__out_r));
And in parallel version
___merge_path_out_lim
__n_1, __n_2, n_out
are used.As far as we already computed these values in
__pattern_merge
, I just pass ones into___merge_path_out_lim
.Also I kept in mind that
__serial_merge_out_lim
may be used (re-called) from the iterator-based merge in the future. That's why I kept iterators in the signature of__serial_merge_out_lim
.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.
The thing is, you do not need
__n_out
for the serial version. In the serial loop, it's not a problem to check on each iteration if the end of the output is reached.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.
..
Basically, only parallel version of ___merge_path_out_lim needs
const _Index3 __n_out = std::min<_Index3>(__n_1 + __n_2, std::ranges::size(__out_r));
logic.So, we can move the mentioned logic inside the function. (and pass iterators from the
__pattern_merge
).