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

Codechange: Remove std::function from Pool iteration wrapper #7911

Conversation

@JGRennison
Copy link
Contributor

JGRennison commented Jan 6, 2020

Remove std::function from Pool iteration wrapper.
Add a separate functor templated wrapper for filtered Pool iteration.

Using std::function results in very poor code generation, and significantly increased code size.
It requires the use of expensive indirect calls, prevents optimisations and inlining, requires various exception and storage overheads, etc.
Trivial functions such as ResetVehicleColourMap() are more than 10x larger in code size than before the change to using PoolIterator instead of macros.
The use of a filter functor even in the unfiltered case creates a significant penalty relative to just checking if the filter is null/empty.

std::function can be removed entirely in the case of non-filtered iteration (the vast majority of cases), and replaced with zero-overhead template functors for filtered iteration, this results in comparable code size and performance to before the change to using PoolIterator.

@LordAro LordAro requested a review from glx22 Jan 6, 2020
{
return Pool::IterateWrapper<Engine>(from, [vt](size_t index) { return Engine::Get(index)->type == vt; });
return Pool::IterateWrapperFiltered<Engine, EngineTypeFilter>(from, EngineTypeFilter{ vt });

This comment has been minimized.

Copy link
@michicc

michicc Jan 6, 2020

Member

Without having actually tried it, but a lambda is supposed to implicitly convert to a functor.

This comment has been minimized.

Copy link
@JGRennison

JGRennison Jan 6, 2020

Author Contributor

Unfortunately you cannot use a lambda as a template parameter, and the full typename needs to be in the return type
Very broadly, lambdas only usefully convert to functors when using them as a parameter to a templated function call.

Copy link
Contributor

glx22 left a comment

You can remove

#include <functional>
as I added it for std::function

Add a separate template wrapper for filtered iteration
@JGRennison JGRennison force-pushed the JGRennison:remove-std-function-pool-iteration-wrapper branch from d25c640 to 17ed94c Jan 7, 2020
@glx22
glx22 approved these changes Jan 7, 2020
@LordAro LordAro merged commit 150dfba into OpenTTD:master Jan 7, 2020
8 checks passed
8 checks passed
OpenTTD CI Build #20200107.1 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.9) Linux linux-amd64-clang-3.9 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.