From 33aedc43a57a7ee203b7e74cf0a5d42ce836c19e Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sat, 27 Apr 2024 08:20:13 +0100 Subject: [PATCH] Codechange: Shrink GUIList vectors less often, reserve before use. After sorting and filter lists for GUI, we often shirnk them to reduce size. However this has very little benefit: 1) The memory has already been allocated, so it doesn't prevent that memory being required. 2) It causes a new allocation and copy when the vector is shrunk, actually using more memory. 3) The list is in window state, so the lifetime is only while the window is open. 4) When a filter is clearer, the original size will be needed again, which will cause another allocation. In fact it is beneficial to reserve to the known maximum in most cases, so do that instead. --- src/build_vehicle_gui.cpp | 2 -- src/company_gui.cpp | 1 - src/group_gui.cpp | 1 - src/industry_gui.cpp | 2 +- src/league_gui.cpp | 2 +- src/network/network_content_gui.cpp | 1 - src/network/network_gui.cpp | 1 - src/newgrf_gui.cpp | 1 - src/object_gui.cpp | 2 +- src/rail_gui.cpp | 2 +- src/road_gui.cpp | 2 +- src/signs_gui.cpp | 2 +- src/station_gui.cpp | 1 - src/story_gui.cpp | 2 -- src/town_gui.cpp | 2 +- 15 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/build_vehicle_gui.cpp b/src/build_vehicle_gui.cpp index c2e0bcc7f6..9b5daa0f43 100644 --- a/src/build_vehicle_gui.cpp +++ b/src/build_vehicle_gui.cpp @@ -1546,7 +1546,6 @@ struct BuildVehicleWindow : Window { case VEH_TRAIN: this->GenerateBuildTrainList(list); GUIEngineListAddChildren(this->eng_list, list); - this->eng_list.shrink_to_fit(); this->eng_list.RebuildDone(); return; case VEH_ROAD: @@ -1584,7 +1583,6 @@ struct BuildVehicleWindow : Window { this->eng_list.swap(list); GUIEngineListAddChildren(this->eng_list, list, INVALID_ENGINE, 0); - this->eng_list.shrink_to_fit(); this->eng_list.RebuildDone(); } diff --git a/src/company_gui.cpp b/src/company_gui.cpp index 586632306b..0c8e58b055 100644 --- a/src/company_gui.cpp +++ b/src/company_gui.cpp @@ -650,7 +650,6 @@ private: BuildGuiGroupList(this->groups, false, owner, vtype); } - this->groups.shrink_to_fit(); this->groups.RebuildDone(); } diff --git a/src/group_gui.cpp b/src/group_gui.cpp index 820d9add0e..64b81228fb 100644 --- a/src/group_gui.cpp +++ b/src/group_gui.cpp @@ -224,7 +224,6 @@ private: BuildGuiGroupList(this->groups, true, owner, this->vli.vtype); - this->groups.shrink_to_fit(); this->groups.RebuildDone(); } diff --git a/src/industry_gui.cpp b/src/industry_gui.cpp index f6d02fefd5..25f48a3ed5 100644 --- a/src/industry_gui.cpp +++ b/src/industry_gui.cpp @@ -1423,6 +1423,7 @@ protected: { if (this->industries.NeedRebuild()) { this->industries.clear(); + this->industries.reserve(Industry::GetNumItems()); for (const Industry *i : Industry::Iterate()) { if (this->string_filter.IsEmpty()) { @@ -1434,7 +1435,6 @@ protected: if (this->string_filter.GetState()) this->industries.push_back(i); } - this->industries.shrink_to_fit(); this->industries.RebuildDone(); auto filter = std::make_pair(this->accepted_cargo_filter_criteria, this->produced_cargo_filter_criteria); diff --git a/src/league_gui.cpp b/src/league_gui.cpp index 60af81e972..062b195212 100644 --- a/src/league_gui.cpp +++ b/src/league_gui.cpp @@ -70,12 +70,12 @@ private: if (!this->companies.NeedRebuild()) return; this->companies.clear(); + this->companies.reserve(Company::GetNumItems()); for (const Company *c : Company::Iterate()) { this->companies.push_back(c); } - this->companies.shrink_to_fit(); this->companies.RebuildDone(); } diff --git a/src/network/network_content_gui.cpp b/src/network/network_content_gui.cpp index f29319331c..a1a8472b76 100644 --- a/src/network/network_content_gui.cpp +++ b/src/network/network_content_gui.cpp @@ -421,7 +421,6 @@ class NetworkContentListWindow : public Window, ContentCallback { this->SetWidgetDisabledState(WID_NCL_SEARCH_EXTERNAL, this->auto_select && all_available); this->FilterContentList(); - this->content.shrink_to_fit(); this->content.RebuildDone(); this->SortContentList(); diff --git a/src/network/network_gui.cpp b/src/network/network_gui.cpp index e108fc2302..9d38926b53 100644 --- a/src/network/network_gui.cpp +++ b/src/network/network_gui.cpp @@ -240,7 +240,6 @@ protected: this->servers.SetFilterState(false); } - this->servers.shrink_to_fit(); this->servers.RebuildDone(); this->vscroll->SetCount(this->servers.size()); diff --git a/src/newgrf_gui.cpp b/src/newgrf_gui.cpp index 47389a152f..7cf2a5283c 100644 --- a/src/newgrf_gui.cpp +++ b/src/newgrf_gui.cpp @@ -1499,7 +1499,6 @@ private: } this->avails.Filter(this->string_filter); - this->avails.shrink_to_fit(); this->avails.RebuildDone(); this->avails.Sort(); diff --git a/src/object_gui.cpp b/src/object_gui.cpp index fc8daaecac..4fec20d2f6 100644 --- a/src/object_gui.cpp +++ b/src/object_gui.cpp @@ -144,6 +144,7 @@ public: if (!this->object_classes.NeedRebuild()) return; this->object_classes.clear(); + this->object_classes.reserve(ObjectClass::GetClassCount()); for (const auto &cls : ObjectClass::Classes()) { if (cls.GetUISpecCount() == 0) continue; // Is this needed here? @@ -151,7 +152,6 @@ public: } this->object_classes.Filter(this->string_filter); - this->object_classes.shrink_to_fit(); this->object_classes.RebuildDone(); this->object_classes.Sort(); diff --git a/src/rail_gui.cpp b/src/rail_gui.cpp index 7c3e2f8c3d..137e78d494 100644 --- a/src/rail_gui.cpp +++ b/src/rail_gui.cpp @@ -1108,6 +1108,7 @@ public: if (!this->station_classes.NeedRebuild()) return; this->station_classes.clear(); + this->station_classes.reserve(StationClass::GetClassCount()); for (const auto &cls : StationClass::Classes()) { /* Skip waypoints. */ @@ -1118,7 +1119,6 @@ public: if (_railstation.newstations) { this->station_classes.Filter(this->string_filter); - this->station_classes.shrink_to_fit(); this->station_classes.RebuildDone(); this->station_classes.Sort(); diff --git a/src/road_gui.cpp b/src/road_gui.cpp index e208aaf36b..efb8cc2713 100644 --- a/src/road_gui.cpp +++ b/src/road_gui.cpp @@ -1263,6 +1263,7 @@ public: if (!this->roadstop_classes.NeedRebuild()) return; this->roadstop_classes.clear(); + this->roadstop_classes.reserve(RoadStopClass::GetClassCount()); for (const auto &cls : RoadStopClass::Classes()) { /* Skip waypoints. */ @@ -1272,7 +1273,6 @@ public: if (this->ShowNewStops()) { this->roadstop_classes.Filter(this->string_filter); - this->roadstop_classes.shrink_to_fit(); this->roadstop_classes.RebuildDone(); this->roadstop_classes.Sort(); diff --git a/src/signs_gui.cpp b/src/signs_gui.cpp index ec2a97c0d4..dbef9641c7 100644 --- a/src/signs_gui.cpp +++ b/src/signs_gui.cpp @@ -63,12 +63,12 @@ struct SignList { Debug(misc, 3, "Building sign list"); this->signs.clear(); + this->signs.reserve(Sign::GetNumItems()); for (const Sign *si : Sign::Iterate()) this->signs.push_back(si); this->signs.SetFilterState(true); this->FilterSignList(); - this->signs.shrink_to_fit(); this->signs.RebuildDone(); } diff --git a/src/station_gui.cpp b/src/station_gui.cpp index 6db2d27cb5..241a512b12 100644 --- a/src/station_gui.cpp +++ b/src/station_gui.cpp @@ -296,7 +296,6 @@ protected: } } - this->stations.shrink_to_fit(); this->stations.RebuildDone(); this->vscroll->SetCount(this->stations.size()); // Update the scrollbar diff --git a/src/story_gui.cpp b/src/story_gui.cpp index 807a4198a7..838bee7f35 100644 --- a/src/story_gui.cpp +++ b/src/story_gui.cpp @@ -77,7 +77,6 @@ protected: } } - this->story_pages.shrink_to_fit(); this->story_pages.RebuildDone(); } @@ -105,7 +104,6 @@ protected: } } - this->story_page_elements.shrink_to_fit(); this->story_page_elements.RebuildDone(); } diff --git a/src/town_gui.cpp b/src/town_gui.cpp index 69af8c411b..0b5e729f99 100644 --- a/src/town_gui.cpp +++ b/src/town_gui.cpp @@ -734,6 +734,7 @@ private: { if (this->towns.NeedRebuild()) { this->towns.clear(); + this->towns.reserve(Town::GetNumItems()); for (const Town *t : Town::Iterate()) { if (this->string_filter.IsEmpty()) { @@ -745,7 +746,6 @@ private: if (this->string_filter.GetState()) this->towns.push_back(t); } - this->towns.shrink_to_fit(); this->towns.RebuildDone(); this->vscroll->SetCount(this->towns.size()); // Update scrollbar as well. }