Codechange: Remove std::function from Pool iteration wrapper #7911
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michicc
reviewed
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 }); |
There was a problem hiding this comment.
Without having actually tried it, but a lambda is supposed to implicitly convert to a functor.
There was a problem hiding this comment.
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.
glx22
requested changes
Jan 6, 2020
There was a problem hiding this comment.
You can remove
OpenTTD/src/core/pool_type.hpp
Line 15 in d25c640
Add a separate template wrapper for filtered iteration
JGRennison
force-pushed
the
remove-std-function-pool-iteration-wrapper
branch
from
d25c640
to
17ed94c
Compare
glx22
approved these changes
Jan 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.