Skip to content

Conversation

achabense
Copy link
Contributor

@achabense achabense commented Sep 3, 2025

This pr removes an unnecessary copy in any::swap.

Hopefully fixes #5698.

Benchmark result:

------------------------------------------------------
Benchmark            Time             CPU   Iterations
------------------------------------------------------
bm<trivial>       14.5 ns         14.8 ns     49777778
bm<small>         21.1 ns         21.0 ns     32000000
bm<large>         15.2 ns         15.0 ns     44800000

->
------------------------------------------------------
Benchmark            Time             CPU   Iterations
------------------------------------------------------
bm<trivial>       10.1 ns         9.52 ns     64000000
bm<small>         15.9 ns         15.3 ns     40727273
bm<large>         10.6 ns         10.7 ns     64000000

@achabense achabense requested a review from a team as a code owner September 3, 2025 16:27
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Sep 3, 2025
@StephanTLavavej StephanTLavavej added the performance Must go faster label Sep 8, 2025
@StephanTLavavej StephanTLavavej self-assigned this Sep 8, 2025
@@ -230,7 +230,10 @@ public:
}

void swap(any& _That) noexcept {
_That = _STD exchange(*this, _STD move(_That));
any _Old = _STD move(*this);
_Assign(_STD move(_That));
Copy link
Contributor Author

@achabense achabense Sep 10, 2025

Choose a reason for hiding this comment

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

We are still paying an additional copy for some insane self-swaps here...

std::any a = std::make_any<std::any>(42);
std::any& a_o = *std::any_cast<std::any>(&a);
a.swap(a_o);
a_o.swap(a);

(Also, I think _Assign(any-prvalue) is somewhat misleading (for example #5413)), and it's better to create the temporal any explicitly in each function. For example, this would be v.)

any _Old_this = _STD move(*this);
{
    any _Old_that = _STD move(_That);
    reset();
    _Move_from(_Old_that);
}
_That.reset();
_That._Move_from(_Old_this);


BENCHMARK(bm<trivial>);
BENCHMARK(bm<small>);
BENCHMARK(bm<large>);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The benchmark is very simple, but I think it's enough to show the compiler cannot optimize away the copy.

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

Successfully merging this pull request may close these issues.

Questions about the precondition of std::any's assignment operators
2 participants