Skip to content

Reduce memory consumption of two ranges tests - #2657

Merged
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
CaseyCarter:shrink-remove_copy
Apr 27, 2022
Merged

Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
CaseyCarter:shrink-remove_copy

Conversation

@CaseyCarter

Copy link
Copy Markdown
Contributor

By making the stateful lambda a namespace-scope function object (at the suggestion of the FE team) and reducing the combinatorics of the test matrix of range properties.

By making the stateful lambda a namespace-scope function object (at the suggestion of the FE team) and reducing the combinatorics of the test matrix of range properties.
@CaseyCarter Casey Carter (CaseyCarter) added test Related to test code ranges C++20/23 ranges labels Apr 16, 2022
@CaseyCarter
Casey Carter (CaseyCarter) requested a review from a team as a code owner April 16, 2022 02:31

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot, i started to also rely more on struct these days

@CaseyCarter Casey Carter (CaseyCarter) changed the title Reduce ranges_alg_remove_copy memory Reduce memory consumption of two ranges tests Apr 19, 2022
@AlexGuteniev

Copy link
Copy Markdown
Contributor

and reducing the combinatorics of the test matrix

Where? 👀

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How much time/mem does the struct conversion alone save? Without running only "interesting" specializations?

Comment thread tests/std/tests/P0896R4_ranges_alg_remove_copy/test.cpp
Comment thread tests/std/tests/P0896R4_ranges_alg_remove_copy/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_alg_remove_copy/test.cpp
@StephanTLavavej

Copy link
Copy Markdown
Member

I have pushed a trivial change to remove unnecessary test:: qualification, which looks more complicated than it is due to (desirable) clang-format reflowing.

@StephanTLavavej

Copy link
Copy Markdown
Member

Casey Carter (@CaseyCarter)

and reducing the combinatorics of the test matrix

Alex Guteniev (@AlexGuteniev)

Where? 👀

I believe that this is happening, not in the meow_matrix.lst, but in replacing the runtime call to test_in_write<instantiator, const P, P>(); (now guarded by TEST_EVERYTHING) with a few specific lines of instantiator::call.

@CaseyCarter

Casey Carter (CaseyCarter) commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

and reducing the combinatorics of the test matrix

Where? 👀

The body of main. The previous body is still there under #ifdef TEST_EVERYTHING. test_in_write:

template <class Instantiator, class Element1, class Element2>
constexpr void test_in_write() {
with_input_ranges<with_writable_iterators<Instantiator, Element2>, Element1>::call();
}

generates calls to instantiator::call with the cross product of "all variations of input range properties" (about 55 cases):

// For all ranges, IsCommon implies Eq.
// For single-pass ranges, Eq is uninteresting without IsCommon (there's only one valid iterator
// value at a time, and no reason to compare it with itself for equality).
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::no, Common::no, CanCompare::no, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::no, Common::no, CanCompare::no, ProxyRef::yes>>();
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::yes>>();
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::yes>>();
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::no, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::yes>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::no, Common::no, CanCompare::no, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::no, Common::no, CanCompare::no, ProxyRef::yes>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::yes>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::yes>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::no>>();
Continuation::template call<Args...,
range<input, Element, Sized::yes, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::yes>>();
with_forward_ranges<Continuation, Element>::template call<Args...>();

and "all variations of writable iterator properties" (about 15 cases):

// Diff and Eq are not significant for "lone" single-pass iterators, so we can ignore them here.
Continuation::template call<Args...,
iterator<input, Element, CanDifference::no, CanCompare::no, ProxyRef::no>>();
Continuation::template call<Args...,
iterator<input, Element, CanDifference::no, CanCompare::no, ProxyRef::yes>>();
with_output_iterators<Continuation, Element>::template call<Args...>();

for a total of 15 * 55 = 825 specializations tested. This is replaced by directly calling instantiator::call five different ways. We know this is adequate coverage only by knowing the details of the implementation.

@CaseyCarter

Casey Carter (CaseyCarter) commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

How much time/mem does the struct conversion alone save?

I only measured remove_copy. (Xiang Fan (@xiangfan-ms) reported similar numbers for remove_copy_if which was motivation enough for me to add it without measuring myself.) Only this change affects the maximum memory size, which went from 5.8GB to 125MB. The corresponding reduction in runtime for all 17 test configs on my 3950X was (IIRC) 90 seconds down to the low twenties.

Without running only "interesting" specializations?

This further reduced total runtime down to about 2.5 seconds.

@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 314f65f into microsoft:main Apr 27, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for dramatically improving the resource consumption of these tests! 🐱 🎉 📉

@Osyotr

Copy link
Copy Markdown

How could this lambda cause this extensive memory consumption?

@miscco

Copy link
Copy Markdown
Contributor

How could this lambda cause this extensive memory consumption?

Every time when we instantiate the function, we create a new stateful lambda. And we instantiate that function a ton

@CaseyCarter
Casey Carter (CaseyCarter) deleted the shrink-remove_copy branch August 21, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ranges C++20/23 ranges test Related to test code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants