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

More optimizations for contiguous iterators #1772

Merged
merged 6 commits into from
Apr 5, 2021

Conversation

AdamBucior
Copy link
Contributor

Changes in this PR:

  • Implemented memmove optimization in ranges::copy/move family
  • Extended memcpy optimization in ranges::uninitialized_copy/move to non-common ranges
  • doing-nothing-for-trivially-destructible-types optimization in ranges::destroy
  • optimizations for non-unwrappable contiguous iterators in everything that depends on _Ptr_*_cat
  • also corrected a small semantic mistake: _Ptr_copy_cat<move_iterator<*>, *> should derive from _Ptr_move_cat

@AdamBucior AdamBucior requested a review from a team as a code owner March 23, 2021 14:49
@AdamBucior

This comment has been minimized.

@CaseyCarter CaseyCarter added the performance Must go faster label Mar 23, 2021
Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

Just a few nitpicky things.

stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter self-assigned this Mar 23, 2021
stl/inc/algorithm Outdated Show resolved Hide resolved
Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

Repeating one comment that was buried in my first review and went unnoticed.

stl/inc/xutility Show resolved Hide resolved
@CaseyCarter CaseyCarter removed their assignment Mar 23, 2021
stl/inc/memory Outdated Show resolved Hide resolved
stl/inc/memory Outdated Show resolved Hide resolved
stl/inc/xmemory Outdated Show resolved Hide resolved
stl/inc/xmemory Outdated Show resolved Hide resolved
stl/inc/xmemory Outdated Show resolved Hide resolved
stl/inc/xmemory Outdated Show resolved Hide resolved
stl/inc/xmemory Outdated Show resolved Hide resolved
stl/inc/xmemory Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
@miscco
Copy link
Contributor

miscco commented Mar 26, 2021

@AdamBucior I think the recommendation was to simply use a 👍 emoji and resolving the comment rather than a reply, when review comments are adopted without change. That helps a lot keeping the inbox clean

@AdamBucior
Copy link
Contributor Author

BTW this is ready for review

@StephanTLavavej StephanTLavavej self-assigned this Mar 30, 2021
@StephanTLavavej StephanTLavavej removed their assignment Apr 2, 2021
@StephanTLavavej StephanTLavavej self-assigned this Apr 2, 2021
@StephanTLavavej StephanTLavavej merged commit cf15e49 into microsoft:main Apr 5, 2021
@StephanTLavavej
Copy link
Member

Thanks for continuing these performance improvements! I'm going to initially Changelog this under "Dev17" 17.0 Preview 1, but we're going to try to port it (along with C++20 feature work) to VS 2019 16.10 Preview 3. 🚀 🚀 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants