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

Add benchsimulate command #10700

Merged
merged 6 commits into from
Jan 10, 2021
Merged

Conversation

janisozaur
Copy link
Member

Adds a new command, benchsimulate, that starts benchmarking
UpdateLogic() function.

@ZehMatt
Copy link
Member

ZehMatt commented Feb 14, 2020

Is there a way to annotate/tag a certain event during the profiling? The UpdateLogic function has like a bunch of sub-updates and it would be nice to know which one of them performs terrible, heres a list:

date_update
scenario_update
map_update_tiles
map_remove_provisional_elements
map_update_path_wide_flags
peep_update_all
map_restore_provisional_elements
vehicle_update_all
sprite_misc_update_all
research_update
ride_ratings_update_all
ride_measurements_update

There are quite a few, so it would be nice to know what is what.

@janisozaur
Copy link
Member Author

I see two ways to provide this:

  1. Add explicit benchmarks for each of those
  • note that testing a single function out of the bunch could lead to results different than the current approach of using UpdateLogic(), as they likely touch same data (think: updating the peeps but not vehicles they're in could lead to unexpected results)
  1. Use manual timing (a feature of gbench) and collect timings for each relevant section
  • this could be reused to expose some timing details in-game (see how it's done in OpenTTD https://imgur.com/gallery/tyI3Htc)
  • collect them to a map so they values can be given names without having to change underlying structure
  • display them using gbench's custom counters

src/openrct2/cmdline/BenchUpdate.cpp Outdated Show resolved Hide resolved
src/openrct2/cmdline/BenchUpdate.cpp Outdated Show resolved Hide resolved
@AaronVanGeffen
Copy link
Member

This is essentially doing the same thing as #10281. We need to decide which to merge.

@tupaschoal tupaschoal added this to the v0.2.6 - Consideration milestone Apr 13, 2020
@tupaschoal tupaschoal modified the milestones: v0.2.6 - Consideration, v0.2.7 Consideration Apr 17, 2020
@Gymnasiast Gymnasiast marked this pull request as draft July 9, 2020 13:04
@janisozaur janisozaur marked this pull request as ready for review December 28, 2020 22:57
Copy link
Member

@tupaschoal tupaschoal left a comment

Choose a reason for hiding this comment

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

This is looking very neat

src/openrct2/GameState.h Outdated Show resolved Hide resolved
src/openrct2/GameState.cpp Show resolved Hide resolved
src/openrct2/GameState.h Outdated Show resolved Hide resolved
src/openrct2/GameState.h Outdated Show resolved Hide resolved
src/openrct2/cmdline/BenchUpdate.cpp Show resolved Hide resolved
src/openrct2/cmdline/BenchUpdate.cpp Show resolved Hide resolved
src/openrct2/cmdline/BenchUpdate.cpp Outdated Show resolved Hide resolved
Comment on lines +60 to +82
state.counters["NetworkUpdateAcc_ms"] = accumulator(LogicTimePart::NetworkUpdate);
state.counters["DateAcc_ms"] = accumulator(LogicTimePart::Date);
state.counters["ScenarioAcc_ms"] = accumulator(LogicTimePart::Scenario);
state.counters["ClimateAcc_ms"] = accumulator(LogicTimePart::Climate);
state.counters["MapTilesAcc_ms"] = accumulator(LogicTimePart::MapTiles);
state.counters["MapStashProvisionalElementsAcc_ms"] = accumulator(LogicTimePart::MapStashProvisionalElements);
state.counters["MapPathWideFlagsAcc_ms"] = accumulator(LogicTimePart::MapPathWideFlags);
state.counters["PeepAcc_ms"] = accumulator(LogicTimePart::Peep);
state.counters["MapRestoreProvisionalElementsAcc_ms"] = accumulator(LogicTimePart::MapRestoreProvisionalElements);
state.counters["VehicleAcc_ms"] = accumulator(LogicTimePart::Vehicle);
state.counters["MiscAcc_ms"] = accumulator(LogicTimePart::Misc);
state.counters["RideAcc_ms"] = accumulator(LogicTimePart::Ride);
state.counters["ParkAcc_ms"] = accumulator(LogicTimePart::Park);
state.counters["ResearchAcc_ms"] = accumulator(LogicTimePart::Research);
state.counters["RideRatingsAcc_ms"] = accumulator(LogicTimePart::RideRatings);
state.counters["RideMeasurmentsAcc_ms"] = accumulator(LogicTimePart::RideMeasurments);
state.counters["NewsAcc_ms"] = accumulator(LogicTimePart::News);
state.counters["MapAnimationAcc_ms"] = accumulator(LogicTimePart::MapAnimation);
state.counters["SoundsAcc_ms"] = accumulator(LogicTimePart::Sounds);
state.counters["GameActionsAcc_ms"] = accumulator(LogicTimePart::GameActions);
state.counters["NetworkFlushAcc_ms"] = accumulator(LogicTimePart::NetworkFlush);
state.counters["ScriptsAcc_ms"] = accumulator(LogicTimePart::Scripts);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should have something that converts from an enum of LogicTimePart into the corresponding string, to be less error prone. That would also allows us to do a for on the enum updating all counters

for (<all_enums>)
    state.counters[ToLogicPartString(enum)] = accumulator(enum);

src/openrct2/cmdline/BenchUpdate.cpp Outdated Show resolved Hide resolved
Copy link
Member

@tupaschoal tupaschoal left a comment

Choose a reason for hiding this comment

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

I've dropped the issues where you commented, but there are still a few to tackle.

@janisozaur
Copy link
Member Author

I've just updated, think it should be ready now

@janisozaur janisozaur merged commit d55bff7 into OpenRCT2:develop Jan 10, 2021
@janisozaur janisozaur deleted the benchsimulate branch January 10, 2021 13:17
@tupaschoal tupaschoal added this to the v0.3.3 milestone Jan 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants