Skip to content

Conversation

@AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented Oct 25, 2025

Towards #5504.

One of the parts of the issue that can be addressed without having copyable_function.

✅ Coverage

Added just copy counter to see that the optimization works. It is copied zero times due to some core language optimization, but without the optimization it is copied once in the cases where it was missing.

Also added allocation counters to see that the other part of the optimization works.

Exercise different wrapping and also null functions.

⚙️ Optimization

Extract the inner _Func_impl / _Func_impl_no_alloc and use directly it without the containing function. This both gets call unwrapping and allocation avoidance.

⚠️ ABI and alignment issues

This change is only complicated in the current ABI. If in a future ABI it is possible to implement function the same way that move_only_function is implemented, things would be way easier.

For 32-bit we can't easily avoid allocation of small function. The buffer of function is aligned to max_align_t and the buffer inside move_only_function is aligned only as void* if it is next to vTable. move_only_function can contain objects of max_align_t alignment, but in that case the buffer size would be smaller. As type erasure already happened by the time we have function, we can't query actual size or alignment.

We probably could change move_only_function to make is emulated vtable the last pointer instead of the first, as we're not ABI-bound for /std:c++latest yet. But it doesn't look like a good idea, considering other, more common move_only_function usage.

❄️ Special cases

We throw bad_function_call, and don't make move_only_function containing null function null itself. Because the Standard is spelled out this way currently.

If function is constructed inside move_only_function using in_place_type, this optimization is not engaged.

Also see #5806

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner October 25, 2025 13:17
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Oct 25, 2025
@StephanTLavavej StephanTLavavej removed their assignment Nov 4, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Work In Progress in STL Code Reviews Nov 4, 2025
@StephanTLavavej
Copy link
Member

Thanks! 😻 I've fully reviewed the product code, pushed a _WIN64 reorganization, and it looks good now. Back to you to enhance the test code until you're happy with it, then send it back to me.

and mitigate possible future combinatorial explosion

not fusing moves and destroys as they are distinct enough
@AlexGuteniev
Copy link
Contributor Author

Back to you to enhance the test code until you're happy with it, then send it back to me.

I've pushed some simplifications to product code, and also expanded test coverage to actually test null functions and count allocations.

@StephanTLavavej StephanTLavavej moved this from Work In Progress to Initial Review in STL Code Reviews Nov 5, 2025
@StephanTLavavej StephanTLavavej added the high priority Important! label Nov 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

high priority Important! performance Must go faster

Projects

Status: Initial Review

Development

Successfully merging this pull request may close these issues.

2 participants