Improve FleetWnd split fleet #863
Merged
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
This will determine how often fleet buttons are regenerated multiple times during a single refresh interval.
Split fleet from the FleetWnd generates one new fleet per friendly ship in a system. Currently, that generates one RefreshFleetButton() call per fleet although only the results from the last RefreshFleetButton() call before the Render() affect the results. This commit creates a DeferredRefreshFleetButton() that is called once if necessary at the start of the next Render() call.
Previously, the code was SetFleetMovementLine(per fleet button) which meant that fleet buttons with multiple fleets updated each fleet multiple times.
FleetsAddedOrRemoved() would remove all of the signals from and reconnect signals for every fleet in the universe. These two new handlers, FleetsInsertedSignalHandler() and FleetsRemovedSignalHandler() only deletes/inserts handlers for the fleets in its input.
By adding boost::hash<TemporaryPtr<T> >, TemporaryPtr<T> can be a member in boost:unordered_set and a key in boost::unordered_map.
Avoid examining every object in Universe and only inspect fleets when creating departing, stationary and moving sets of fleets.
Compare new sets with old sets.
MapWnd::CreateFleetButtonsOfType() creates fleet buttons and stores them in m_stationary_, m_departing_ or m_moving_ fleet_buttons as appropriate.
Now that the deferred update runs at most once per render this is always less than one.
|
For reviewers: I've tested this and it works fine on my system, with a noticeable improvement in performance, I cannot comment on the code itself but would recommend inclusion ASAP if possible. |
| for (std::set<FleetButton*>::iterator it = m_moving_fleet_buttons.begin(); it != m_moving_fleet_buttons.end(); ++it) { | ||
| FleetButton* fb = *it; | ||
| for (boost::unordered_map<std::pair<double, double>, boost::unordered_set<FleetButton*> >::iterator pos_it = m_moving_fleet_buttons.begin(); | ||
| pos_it != m_moving_fleet_buttons.end(); ++pos_it) { |
| for (std::set<FleetButton*>::iterator button_it = set.begin(); button_it != set.end(); ++button_it) | ||
| for (boost::unordered_map<int, boost::unordered_set<FleetButton*> >::iterator it = m_departing_fleet_buttons.begin(); it != m_departing_fleet_buttons.end(); ++it) { | ||
| boost::unordered_set<FleetButton*>& set = it->second; | ||
| for (boost::unordered_set<FleetButton*>::iterator button_it = set.begin(); button_it != set.end(); ++button_it) |
1. Style.
|
@geoffthemedio thanks for the prompt review. I've made all the requested changes. |
Vezzra
added
the
category:refactoring
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.
This PR improves the run time of
FleetWndsplit fleet.It will also improve update times for any maps which contain fleets.
It does not change any game rules, or UI elements. I think it is suitable for inclusion in 0.4.6.
On my test example merging all the fleets in a fleet button with many fleets and then splitting them. The splitting took 19 seconds before this PR and 3 seconds after.
You can compare the results yourself with any save with a large fleet. The PR is save compatible.
The first commit efa00c3 contains the
FleetWnd::SplitFleetScopedTimerfor split fleet. If you checkout efa00c3 and split a big fleet, the time will be in thefreeorion.logasFleetWnd::SplitFleet time: xxxx.xx. You can compare the results with the whole PR. The changes are large enough to be noticeable without the timer.The PR makes several changes in
MapWndthat cumulatively produce the improvement.DeferredRefreshFleetButtons()run only if needed at the start ofRender()It makes some code quality changes, by reducing repeated code.