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

Optimize item spawn list creation #55099

Merged
merged 1 commit into from
Feb 5, 2022

Conversation

BrettDong
Copy link
Member

@BrettDong BrettDong commented Feb 4, 2022

Summary

Performance "Optimize item spawn list creation"

Purpose of change

before

Around 7% of time in starting_items test case is just spent on concatenating std::vector<item>.

Current implementation of creating item spawn list is recursively doing

std::vector<item> create(...) {
    std::vector<item> result;
    std::vector<item> temp = create(...);
    result.insert( result.end(), temp.begin(), temp.end() );
    return result;
}

This results in a lot of temporary item and std::vector<item> construction, and std::vector<item> range copying.

Describe the solution

Instead of returning std::vector<item> container in the recursive algorithm, pass around the result std::vector<item> as a mutable reference in the parameters of the recursive algorithm, and construct items in place in the one std::vector<item> container.

Describe alternatives you've considered

Refactor the recursive algorithm to something iterative.

Testing

after

std::vector<item> data copy overhead is completely resolved. Reduces starting_items test time by 12 seconds on my computer. This optimization also benefits mapgen slightly.

Additional context

@BrettDong BrettDong added [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) Spawn Creatures, items, vehicles, locations appearing on map labels Feb 4, 2022
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Feb 4, 2022
@akrieger
Copy link
Member

akrieger commented Feb 4, 2022

Nice fix. I noticed this but didn't know enough about the item code to know how to approach this.

@BrettDong
Copy link
Member Author

Your #55070 inspired me to look into inefficient use of temporary container concatenation.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Feb 4, 2022
@kevingranade kevingranade merged commit 94dfb94 into CleverRaven:master Feb 5, 2022
@BrettDong BrettDong deleted the item_spawn branch February 5, 2022 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) json-styled JSON lint passed, label assigned by github actions Spawn Creatures, items, vehicles, locations appearing on map
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants