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

Mitigate overhead of walking all loaded submaps for vehicle processing. #36913

Conversation

kevingranade
Copy link
Member

The code is inappropriately walking all loaded submaps in order to process vehicle power production and consumption.
This mitigates the overhead by skipping most of the processing in the common case when a submap has no vehicle.

Summary

SUMMARY: Bugfixes "Mitigate out of control vehicle processing overhead."

Purpose of change

As reported in #36453 the game is suffering from increasing per-turn overhead the more map area is explored between saves.
The cause of this is this chunk of code that updates idle state.

Cataclysm-DDA/src/game.cpp

Lines 1520 to 1531 in 64b5e04

for( auto &elem : MAPBUFFER ) {
tripoint sm_loc = elem.first;
point sm_topleft = sm_to_ms_copy( sm_loc.xy() );
point in_reality = m.getlocal( sm_topleft );
submap *sm = elem.second;
const bool in_bubble_z = m.has_zlevels() || sm_loc.z == get_levz();
for( auto &veh : sm->vehicles ) {
veh->idle( in_bubble_z && m.inbounds( in_reality ) );
}
}

Describe the solution

This partially mitigates the problem by doing a very cheap check to skip most (I think about 80%) of the overhead caused by this issue, the remaining work is simply iterating over the tree.

Describe alternatives you've considered

I need to completely replace this mechanism with a list of vehicles on the map plus vehicles connected to them, which will not scale in the same way.

Testing

Insure vehicles are still processed, i.e. fuel and battery consumption.

The code is inappropriately walking all loaded submaps in order to process vehicle power production and consumption.
This mitigates the overhead by skipping most of the processing in the common case when a submap has no vehicle.
@ZhilkinSerg ZhilkinSerg added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) Vehicles Vehicles, parts, mechanics & interactions labels Jan 11, 2020
@@ -1518,6 +1518,9 @@ bool game::do_turn()
// Process power and fuel consumption for all vehicles, including off-map ones.
// m.vehmove used to do this, but now it only give them moves instead.
for( auto &elem : MAPBUFFER ) {
if( sm->vehicles.empty() ) {
Copy link
Contributor

@ZhilkinSerg ZhilkinSerg Jan 11, 2020

Choose a reason for hiding this comment

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

sm is only initialized several lines below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just as well I have a better fix ready.

@kevingranade kevingranade deleted the mitigate-vehicle-processing-overhead branch June 28, 2020 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants