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

vehicle: improve performance by removing has_part( "ROTOR" ) calls #44941

Merged
merged 1 commit into from Oct 19, 2020

Conversation

mlangsdorf
Copy link
Contributor

Summary

SUMMARY: Performance "vehicle: improve performance by removing has_part( "ROTOR" ) calls"

Purpose of change

has_part( "ROTOR" ) checks every part in a vehicle to see if the part is a rotor. This is ridiculously expensive in compute time and unnecessary, because the presence of rotors is cached in vehicle::rotors which is automatically refreshed in vehicle::refresh().

Profiling was showing that 6% of overall compute time was spent has_rotor( "ROTOR" ) calls.

Describe the solution

Replace the calls to ( has_part( "ROTOR" ) && has_part( "ROTOR_SIMPLE" ) ) with !rotors.empty() for a noticeable performance boost.

Describe alternatives you've considered

More caching could be done in vehicle::check_falling_or_floating() but I think this is the big pay-off.

Testing

Ran the unit tests successfully.

has_part( "ROTOR" ) checks every part in a vehicle to see if the
part is a rotor.  This is ridiculously expensive in compute time and
unnecessary, because the presense of rotors is cached in
vehicle::rotors which is automatically refreshed in vehicle::refresh.

Replace the calls to ( has_part( "ROTOR" ) && has_part( "ROTOR_SIMPLE" ) )
with !rotors.empty() for a noticeable performance boost.
@mlangsdorf mlangsdorf added [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) Vehicles Vehicles, parts, mechanics & interactions labels Oct 18, 2020
@kevingranade kevingranade merged commit f322301 into CleverRaven:master Oct 19, 2020
@mlangsdorf mlangsdorf deleted the helicopter_perf_fix branch October 19, 2020 11:50
@@ -4256,7 +4256,7 @@ int vehicle::get_z_change() const

bool vehicle::would_prevent_flyable( const vpart_info &vpinfo ) const
{
return is_flyable() && has_part( "ROTOR" ) && !vpinfo.has_flag( "SIMPLE_PART" );
Copy link
Contributor

Choose a reason for hiding this comment

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

This should still check for the string ROTOR flag, this message shouldn't be triggering when it's a SIMPLE_ROTOR. The other change below this is potentially the same deal (though that could probably check !rotors.empty() then has_part( ROTOR ) for performance).

@kevingranade
Copy link
Member

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/vehicle-question-spooilers-aftershock/24462/23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[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.

None yet

3 participants