Improve performance of name sorting in town and station list windows

src/base_station_base.h

@@ -17,6 +17,8 @@
 #include "viewport_type.h"
 #include "station_map.h"
 #include "core/geometry_type.hpp"
+#include "core/alloc_type.hpp"
+#include <memory>
 
 typedef Pool<BaseStation, StationID, 32, 64000> StationPool;
 extern StationPool _station_pool;
@@ -59,6 +61,7 @@ struct BaseStation : StationPool::PoolItem<&_station_pool> {
 
 	char *name;                     ///< Custom name
 	StringID string_id;             ///< Default name (town area) of station
+	std::unique_ptr<const char, FreeDeleter> cached_name; ///< NOSAVE: Cache of the resolved name of the station, if not using a custom name
 
 	Town *town;                     ///< The town this station is associated with
 	OwnerByte owner;                ///< The owner of this station
@@ -113,6 +116,13 @@ struct BaseStation : StationPool::PoolItem<&_station_pool> {
 	 */
 	virtual void UpdateVirtCoord() = 0;
 
+	inline const char *GetCachedName() const
+	{
+		if (this->name != nullptr) return this->name;
+		if (!this->cached_name) const_cast<BaseStation *>(this)->FillCachedName();
+		return this->cached_name.get();
+	}
+
 	virtual void MoveSign(TileIndex new_xy)
 	{
 		this->xy = new_xy;
@@ -176,6 +186,9 @@ struct BaseStation : StationPool::PoolItem<&_station_pool> {
 	}
 
 	static void PostDestructor(size_t index);
+
+	private:
+	void FillCachedName();
 };
 
 #define FOR_ALL_BASE_STATIONS(var) FOR_ALL_ITEMS_FROM(BaseStation, station_index, var, 0)

src/saveload/afterload.cpp

@@ -232,6 +232,12 @@ void UpdateAllVirtCoords()
 	RebuildViewportKdtree();
 }
 
+void ClearAllCachedNames()
+{
+	ClearAllStationCachedNames();
+	ClearAllTownCachedNames();
+}
+
 /**
  * Initialization of the windows and several kinds of caches.
  * This is not done directly in AfterLoadGame because these
@@ -248,6 +254,7 @@ static void InitializeWindowsAndCaches()
 	SetupColoursAndInitialWindow();
 
 	/* Update coordinates of the signs. */
+	ClearAllCachedNames();
 	UpdateAllVirtCoords();
 	ResetViewportAfterLoadGame();
 

src/settings_gui.cpp

@@ -527,6 +527,7 @@ struct GameOptionsWindow : Window {
 				ReadLanguagePack(&_languages[index]);
 				DeleteWindowByClass(WC_QUERY_STRING);
 				CheckForMissingGlyphs();
+				ClearAllCachedNames();
 				UpdateAllVirtCoords();
 				ReInitAllWindows();
 				break;

src/station_cmd.cpp

@@ -456,6 +456,26 @@ void UpdateAllStationVirtCoords()
 	}
 }
 
+void BaseStation::FillCachedName()
+{
+	char buf[256];
+	int64 args_array[] = { this->index };
+	StringParameters tmp_params(args_array);
+	char *end = GetStringWithArgs(buf, Waypoint::IsExpected(this) ? STR_WAYPOINT_NAME : STR_STATION_NAME, &tmp_params, lastof(buf));
+	char *alloced = MallocT<char>(end - buf + 1);
+	memcpy(alloced, buf, end - buf + 1);
+	this->cached_name.reset(alloced);
+}
+
+void ClearAllStationCachedNames()
+{
+	BaseStation *st;
+
+	FOR_ALL_BASE_STATIONS(st) {
+		st->cached_name.reset();
+	}
+}
+
 /**
  * Get a mask of the cargo types that the station accepts.
  * @param st Station to query
@@ -4006,6 +4026,7 @@ CommandCost CmdRenameStation(TileIndex tile, DoCommandFlag flags, uint32 p1, uin
 	}
 
 	if (flags & DC_EXEC) {
+		st->cached_name.reset();
 		free(st->name);
 		st->name = reset ? nullptr : stredup(text);
 

src/station_func.h

@@ -27,6 +27,7 @@ void FindStationsAroundTiles(const TileArea &location, StationList *stations, bo
 
 void ShowStationViewWindow(StationID station);
 void UpdateAllStationVirtCoords();
+void ClearAllStationCachedNames();
 
 CargoArray GetProductionAroundTiles(TileIndex tile, int w, int h, int rad);
 CargoArray GetAcceptanceAroundTiles(TileIndex tile, int w, int h, int rad, CargoTypes *always_accepted = nullptr);

src/station_gui.cpp

@@ -213,19 +213,7 @@ protected:
 	/** Sort stations by their name */
 	static bool StationNameSorter(const Station * const &a, const Station * const &b)
 	{
-		static char buf_cache[64];
-		char buf[64];
-
-		SetDParam(0, a->index);
-		GetString(buf, STR_STATION_NAME, lastof(buf));
-
-		if (b != last_station) {
-			last_station = b;
-			SetDParam(0, b->index);
-			GetString(buf_cache, STR_STATION_NAME, lastof(buf_cache));
-		}
-
-		int r = strnatcmp(buf, buf_cache); // Sort by name (natural sorting).
+		int r = strnatcmp(a->GetCachedName(), b->GetCachedName()); // Sort by name (natural sorting).
 		if (r == 0) return a->index < b->index;
 		return r < 0;
 	}
@@ -1177,21 +1165,13 @@ bool CargoSorter::SortCount(const CargoDataEntry *cd1, const CargoDataEntry *cd2
 
 bool CargoSorter::SortStation(StationID st1, StationID st2) const
 {
-	static char buf1[MAX_LENGTH_STATION_NAME_CHARS];
-	static char buf2[MAX_LENGTH_STATION_NAME_CHARS];
-
 	if (!Station::IsValidID(st1)) {
 		return Station::IsValidID(st2) ? this->order == SO_ASCENDING : this->SortId(st1, st2);
 	} else if (!Station::IsValidID(st2)) {
 		return order == SO_DESCENDING;
 	}
 
-	SetDParam(0, st1);
-	GetString(buf1, STR_STATION_NAME, lastof(buf1));
-	SetDParam(0, st2);
-	GetString(buf2, STR_STATION_NAME, lastof(buf2));
-
-	int res = strnatcmp(buf1, buf2); // Sort by name (natural sorting).
+	int res = strnatcmp(Station::Get(st1)->GetCachedName(), Station::Get(st2)->GetCachedName()); // Sort by name (natural sorting).
 	if (res == 0) {
 		return this->SortId(st1, st2);
 	} else {

src/town.h

@@ -21,7 +21,9 @@
 #include "openttd.h"
 #include "table/strings.h"
 #include "company_func.h"
+#include "core/alloc_type.hpp"
 #include <list>
+#include <memory>
 
 template <typename T>
 struct BuildingCounts {
@@ -65,6 +67,7 @@ struct Town : TownPool::PoolItem<&_town_pool> {
 	uint16 townnametype;
 	uint32 townnameparts;
 	char *name;                    ///< Custom town name. If nullptr, the town was not renamed and uses the generated name.
+	std::unique_ptr<const char, FreeDeleter> cached_name; ///< NOSAVE: Cache of the resolved name of the town, if not using a custom town name
 
 	byte flags;                    ///< See #TownFlags.
 
@@ -158,6 +161,13 @@ struct Town : TownPool::PoolItem<&_town_pool> {
 
 	void UpdateVirtCoord();
 
+	inline const char *GetCachedName() const
+	{
+		if (this->name != nullptr) return this->name;
+		if (!this->cached_name) const_cast<Town *>(this)->FillCachedName();
+		return this->cached_name.get();
+	}
+
 	static inline Town *GetByTile(TileIndex tile)
 	{
 		return Town::Get(GetTownIndex(tile));
@@ -165,11 +175,15 @@ struct Town : TownPool::PoolItem<&_town_pool> {
 
 	static Town *GetRandom();
 	static void PostDestructor(size_t index);
+
+	private:
+	void FillCachedName();
 };
 
 uint32 GetWorldPopulation();
 
 void UpdateAllTownVirtCoords();
+void ClearAllTownCachedNames();
 void ShowTownViewWindow(TownID town);
 void ExpandTown(Town *t);
 

src/town_cmd.cpp

@@ -229,6 +229,15 @@ void Town::UpdateLabel()
 	}
 }
 
+void Town::FillCachedName()
+{
+	char buf[256];
+	char *end = GetTownName(buf, this, lastof(buf));
+	char *alloced = MallocT<char>(end - buf + 1);
+	memcpy(alloced, buf, end - buf + 1);
+	this->cached_name.reset(alloced);
+}
+
 /**
  * Get the cost for removing this house
  * @return the cost (inflation corrected etc)
@@ -522,6 +531,15 @@ void UpdateAllTownVirtCoords()
 	}
 }
 
+void ClearAllTownCachedNames()
+{
+	Town *t;
+
+	FOR_ALL_TOWNS(t) {
+		t->cached_name.reset();
+	}
+}
+
 /**
  * Change the towns population
  * @param t Town which population has changed
@@ -2863,11 +2881,13 @@ CommandCost CmdRenameTown(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32
 	}
 
 	if (flags & DC_EXEC) {
+		t->cached_name.reset();
 		free(t->name);
 		t->name = reset ? nullptr : stredup(text);
 
 		t->UpdateVirtCoord();
 		InvalidateWindowData(WC_TOWN_DIRECTORY, 0, 1);
+		ClearAllStationCachedNames();
 		UpdateAllStationVirtCoords();
 	}
 	return CommandCost();

src/town_gui.cpp

@@ -675,22 +675,7 @@ private:
 	/** Sort by town name */
 	static bool TownNameSorter(const Town * const &a, const Town * const &b)
 	{
-		static char buf_cache[64];
-		char buf[64];
-
-		SetDParam(0, a->index);
-		GetString(buf, STR_TOWN_NAME, lastof(buf));
-
-		/* If 'b' is the same town as in the last round, use the cached value
-		 * We do this to speed stuff up ('b' is called with the same value a lot of
-		 * times after each other) */
-		if (b != last_town) {
-			last_town = b;
-			SetDParam(0, b->index);
-			GetString(buf_cache, STR_TOWN_NAME, lastof(buf_cache));
-		}
-
-		return strnatcmp(buf, buf_cache) < 0; // Sort by name (natural sorting).
+		return strnatcmp(a->GetCachedName(), b->GetCachedName()) < 0; // Sort by name (natural sorting).
 	}
 
 	/** Sort by population (default descending, as big towns are of the most interest). */

src/viewport_func.h

@@ -87,6 +87,7 @@ bool ScrollMainWindowToTile(TileIndex tile, bool instant = false);
 bool ScrollMainWindowTo(int x, int y, int z = -1, bool instant = false);
 
 void UpdateAllVirtCoords();
+void ClearAllCachedNames();
 
 extern Point _tile_fract_coords;