Skip to content

Commit 64bb0a0

Browse files
committed
perf: Improve tech screen init latency by iterating all objects once, instead of all objects per tech, to find dependencies
1 parent 38353bb commit 64bb0a0

5 files changed

Lines changed: 130 additions & 20 deletions

File tree

‎build.gradle.kts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ allprojects {
7474
"io.ktor.http.Parameters.get",
7575

7676
"java.util.BitSet.clone",
77+
78+
"kotlin.collections.orEmpty",
7779
)
7880
wellKnownPureClasses = setOf(
7981
)

‎core/src/com/unciv/ui/images/ImageGetter.kt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import com.unciv.ui.screens.basescreen.BaseScreen
3131
import com.unciv.utils.Concurrency
3232
import com.unciv.utils.debug
3333
import kotlinx.coroutines.runBlocking
34+
import yairm210.purity.annotations.Readonly
3435
import kotlin.math.atan2
3536
import kotlin.math.max
3637
import kotlin.math.min
@@ -224,9 +225,11 @@ object ImageGetter {
224225
fun getExternalImage(fileName: String) =
225226
getExternalImage(Gdx.files.internal("ExtraImages/$fileName"))
226227

227-
fun getImage(fileName: String?, tintColor: Color? = null): Image =
228+
@Readonly @Suppress("purity") // only mutates the freshly-created Image it returns
229+
fun getImage(fileName: String?, tintColor: Color? = null): Image =
228230
ImageWithCustomSize(getDrawable(fileName)).apply { color = tintColor ?: Color.WHITE }
229231

232+
@Readonly
230233
fun getDrawable(fileName: String?): TextureRegionDrawable =
231234
textureRegionDrawables[fileName] ?: textureRegionDrawables[whiteDotLocation]!!
232235

@@ -251,6 +254,7 @@ object ImageGetter {
251254
fun imageExists(fileName: String) = textureRegionDrawables.containsKey(fileName)
252255
fun ninePatchImageExists(fileName: String) = ninePatchDrawables.containsKey(fileName)
253256

257+
@Readonly @Suppress("purity") // only mutates the freshly-created Image it returns
254258
fun getStatIcon(statName: String, size: Float = 20f): Image = getImage("StatIcons/$statName")
255259
.apply { setSize(size, size) }
256260

@@ -267,6 +271,7 @@ object ImageGetter {
267271
getImage("UnitIcons/${unit.name}").apply { this.color = color }
268272
else getImage("UnitTypeIcons/${unit.type}").apply { this.color = color }
269273

274+
@Readonly @Suppress("purity") // only mutates the freshly-created Group it returns
270275
fun getConstructionPortrait(construction: String, size: Float): Group {
271276
if (ruleset.buildings.containsKey(construction)) {
272277
return PortraitBuilding(construction, size)
@@ -279,15 +284,18 @@ object ImageGetter {
279284
return getStatIcon(construction).surroundWithCircle(size).surroundWithThinCircle()
280285
}
281286

287+
@Readonly
282288
fun getUniquePortrait(uniqueName: String, size: Float): Group = PortraitUnique(uniqueName, size)
283289

284290
fun getPromotionPortrait(promotionName: String, size: Float = 30f): Group = PortraitPromotion(promotionName, size)
285291

292+
@Readonly
286293
fun getResourcePortrait(resourceName: String, size: Float, amount: Int= 0): Group =
287294
PortraitResource(resourceName, size, amount)
288295

289296
fun getTechIconPortrait(techName: String, circleSize: Float): Group = PortraitTech(techName, circleSize)
290297

298+
@Readonly
291299
fun getImprovementPortrait(improvementName: String, size: Float = 20f, isPillaged: Boolean = false): Portrait =
292300
PortraitImprovement(improvementName, size, false, isPillaged)
293301

@@ -328,6 +336,7 @@ object ImageGetter {
328336

329337
fun getTriangle() = getImage("OtherIcons/Triangle")
330338

339+
@Readonly @Suppress("purity") // only mutates the freshly-created Actor it returns
331340
fun getRedCross(size: Float, alpha: Float): Actor {
332341
val redCross = getImage("OtherIcons/Close")
333342
redCross.setSize(size, size)

‎core/src/com/unciv/ui/objectdescriptions/TechnologyDescriptions.kt‎

Lines changed: 110 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import com.unciv.ui.images.PortraitUnavailableWonderForTechTree
1818
import com.unciv.ui.screens.civilopediascreen.FormattedLine
1919
import com.unciv.ui.screens.civilopediascreen.ICivilopediaText
2020
import com.unciv.ui.screens.pickerscreens.TechButton
21+
import yairm210.purity.annotations.Readonly
2122

2223

2324
object TechnologyDescriptions {
@@ -89,16 +90,25 @@ object TechnologyDescriptions {
8990

9091
/**
9192
* Gets icons to display on a [TechButton] - all should be also described in [getDescription]
93+
*
94+
* @param iconsIndex Optional precomputed [TechIconsIndex] (see [buildTechIconsIndex]) - when building
95+
* icons for every tech in the ruleset at once (as [com.unciv.ui.screens.pickerscreens.TechPickerScreen] does),
96+
* pass one in instead of letting each call re-scan the whole ruleset - see #15641.
9297
*/
93-
fun getTechEnabledIcons(tech: Technology, viewingCiv: Civilization, techIconSize: Float) = sequence {
98+
@Readonly
99+
fun getTechEnabledIcons(tech: Technology, viewingCiv: Civilization, techIconSize: Float, iconsIndex: TechIconsIndex? = null) = sequence {
94100
val ruleset = viewingCiv.gameInfo.ruleset
95101
val techName = tech.name
96102

97-
for (unit in getEnabledUnits(techName, ruleset, viewingCiv)) {
103+
val enabledUnits = if (iconsIndex != null) iconsIndex.enabledUnitsByTech[techName].orEmpty().asSequence()
104+
else getEnabledUnits(techName, ruleset, viewingCiv)
105+
for (unit in enabledUnits) {
98106
yield(ImageGetter.getConstructionPortrait(unit.name, techIconSize))
99107
}
100108

101-
for (building in getEnabledBuildings(techName, ruleset, viewingCiv)) {
109+
val enabledBuildings = if (iconsIndex != null) iconsIndex.enabledBuildingsByTech[techName].orEmpty().asSequence()
110+
else getEnabledBuildings(techName, ruleset, viewingCiv)
111+
for (building in enabledBuildings) {
102112
// We don't need to show the unavailable marker for techs that are already researched
103113
// since this is mostly a feature to choose which technologies to research.
104114
if (building.isWonder && !viewingCiv.tech.isResearched(techName)) {
@@ -119,26 +129,29 @@ object TechnologyDescriptions {
119129
}
120130
}
121131

122-
yieldAll(
123-
getObsoletedObjects(techName, ruleset, viewingCiv)
124-
.mapNotNull { it.getObsoletedIcon(techIconSize) }
125-
)
132+
val obsoletedObjects = if (iconsIndex != null) iconsIndex.obsoletedObjectsByTech[techName].orEmpty().asSequence()
133+
else getObsoletedObjects(techName, ruleset, viewingCiv)
134+
yieldAll(obsoletedObjects.mapNotNull { it.getObsoletedIcon(techIconSize) })
126135

127-
for (resource in ruleset.tileResources.values.filter { it.revealedBy == techName }) {
136+
val revealedResources = if (iconsIndex != null) iconsIndex.resourcesByRevealedTech[techName].orEmpty().asSequence()
137+
else ruleset.tileResources.values.asSequence().filter { it.revealedBy == techName }
138+
for (resource in revealedResources) {
128139
yield(ImageGetter.getResourcePortrait(resource.name, techIconSize))
129140
}
130141

131-
for (improvement in ruleset.tileImprovements.values.asSequence()
132-
.filter { it.techRequired == techName }
133-
.filter { it.uniqueTo == null || viewingCiv.matchesFilter(it.uniqueTo!!) }
134-
) {
142+
val requiredTechImprovements = if (iconsIndex != null) iconsIndex.improvementsByTechRequired[techName].orEmpty().asSequence()
143+
else ruleset.tileImprovements.values.asSequence()
144+
.filter { it.techRequired == techName }
145+
.filter { it.uniqueTo == null || viewingCiv.matchesFilter(it.uniqueTo!!) }
146+
for (improvement in requiredTechImprovements) {
135147
yield(ImageGetter.getImprovementPortrait(improvement.name, techIconSize))
136148
}
137149

138-
for (improvement in ruleset.tileImprovements.values.asSequence()
139-
.filter { it.uniqueObjects.any { u -> u.allParams.contains(techName) } }
140-
.filter { it.uniqueTo == null || viewingCiv.matchesFilter(it.uniqueTo!!) }
141-
) {
150+
val uniqueParamImprovements = if (iconsIndex != null) iconsIndex.improvementsByUniqueTechParam[techName].orEmpty().asSequence()
151+
else ruleset.tileImprovements.values.asSequence()
152+
.filter { it.uniqueObjects.any { u -> u.allParams.contains(techName) } }
153+
.filter { it.uniqueTo == null || viewingCiv.matchesFilter(it.uniqueTo!!) }
154+
for (improvement in uniqueParamImprovements) {
142155
yield(ImageGetter.getUniquePortrait(improvement.name, techIconSize))
143156
}
144157

@@ -155,6 +168,82 @@ object TechnologyDescriptions {
155168
}
156169
}
157170

171+
/**
172+
* Precomputes, in one pass over the ruleset, which units/buildings/resources/improvements
173+
* are unlocked or obsoleted by each tech - grouped by tech name for O(1) lookup in [getTechEnabledIcons].
174+
*
175+
* Without this, opening [com.unciv.ui.screens.pickerscreens.TechPickerScreen] re-scanned the entire
176+
* ruleset (units, buildings, resources, improvements) once per tech - O(techs * rulesetObjects) total,
177+
* which was the dominant cost of constructing that screen and a contributor to a black-screen flash
178+
* on Android when it ran synchronously on the GL thread (#15641).
179+
*/
180+
@Readonly @Suppress("purity") // only mutates the locally-built index maps/lists it returns
181+
fun buildTechIconsIndex(ruleset: Ruleset, viewingCiv: Civilization): TechIconsIndex {
182+
val filteredBuildings = getFilteredBuildings(ruleset, viewingCiv) { true }.toList()
183+
val enabledBuildingsByTech = HashMap<String, MutableList<Building>>()
184+
for (building in filteredBuildings)
185+
for (tech in building.requiredTechs())
186+
enabledBuildingsByTech.getOrPut(tech) { mutableListOf() }.add(building)
187+
188+
val filteredUnits = ruleset.units.values.asSequence()
189+
.filter {
190+
(it.uniqueTo != null && viewingCiv.matchesFilter(it.uniqueTo!!) ||
191+
it.uniqueTo == null && viewingCiv.getEquivalentUnit(it) == it)
192+
&& !it.isHiddenFromCivilopedia(ruleset)
193+
}.toList()
194+
val enabledUnitsByTech = HashMap<String, MutableList<BaseUnit>>()
195+
for (unit in filteredUnits)
196+
for (tech in unit.requiredTechs())
197+
enabledUnitsByTech.getOrPut(tech) { mutableListOf() }.add(unit)
198+
199+
val filteredImprovements = ruleset.tileImprovements.values.asSequence()
200+
.filter { it.uniqueTo == null || viewingCiv.matchesFilter(it.uniqueTo!!) }
201+
.toList()
202+
203+
val obsoletedObjectsByTech = HashMap<String, MutableList<RulesetStatsObject>>()
204+
val obsoletionCandidates: Sequence<RulesetStatsObject> =
205+
filteredBuildings.asSequence() + ruleset.tileResources.values.asSequence() + filteredImprovements.asSequence()
206+
for (obj in obsoletionCandidates)
207+
for (unique in obj.getMatchingUniques(UniqueType.ObsoleteWith))
208+
obsoletedObjectsByTech.getOrPut(unique.params[0]) { mutableListOf() }.add(obj)
209+
210+
val resourcesByRevealedTech = HashMap<String, MutableList<TileResource>>()
211+
for (resource in ruleset.tileResources.values) {
212+
val revealedBy = resource.revealedBy ?: continue
213+
resourcesByRevealedTech.getOrPut(revealedBy) { mutableListOf() }.add(resource)
214+
}
215+
216+
val improvementsByTechRequired = HashMap<String, MutableList<TileImprovement>>()
217+
for (improvement in filteredImprovements) {
218+
val techRequired = improvement.techRequired ?: continue
219+
improvementsByTechRequired.getOrPut(techRequired) { mutableListOf() }.add(improvement)
220+
}
221+
222+
val improvementsByUniqueTechParam = HashMap<String, LinkedHashSet<TileImprovement>>()
223+
for (improvement in filteredImprovements)
224+
for (unique in improvement.uniqueObjects)
225+
for (param in unique.allParams)
226+
improvementsByUniqueTechParam.getOrPut(param) { LinkedHashSet() }.add(improvement)
227+
228+
return TechIconsIndex(
229+
enabledUnitsByTech,
230+
enabledBuildingsByTech,
231+
obsoletedObjectsByTech,
232+
resourcesByRevealedTech,
233+
improvementsByTechRequired,
234+
improvementsByUniqueTechParam.mapValues { it.value.toList() }
235+
)
236+
}
237+
238+
class TechIconsIndex internal constructor(
239+
val enabledUnitsByTech: Map<String, List<BaseUnit>>,
240+
val enabledBuildingsByTech: Map<String, List<Building>>,
241+
val obsoletedObjectsByTech: Map<String, List<RulesetStatsObject>>,
242+
val resourcesByRevealedTech: Map<String, List<TileResource>>,
243+
val improvementsByTechRequired: Map<String, List<TileImprovement>>,
244+
val improvementsByUniqueTechParam: Map<String, List<TileImprovement>>
245+
)
246+
158247
/**
159248
* Implementation of [ICivilopediaText.getCivilopediaTextLines]
160249
*/
@@ -273,6 +362,7 @@ object TechnologyDescriptions {
273362
* nuclear weapons and religion settings, and without those expressly hidden from Civilopedia.
274363
*/
275364
// Used for Civilopedia, Alert and Picker, so if any of these decide to ignore the "Will not be displayed in Civilopedia" unique this needs refactoring
365+
@Readonly
276366
private fun getEnabledBuildings(techName: String, ruleset: Ruleset, civInfo: Civilization?) =
277367
getFilteredBuildings(ruleset, civInfo) { it.requiredTechs().contains(techName) }
278368

@@ -281,6 +371,7 @@ object TechnologyDescriptions {
281371
* nuclear weapons and religion settings, and without those expressly hidden from Civilopedia.
282372
*/
283373
// Used for Civilopedia, Alert and Picker, so if any of these decide to ignore the "Will not be displayed in Civilopedia" unique this needs refactoring
374+
@Readonly
284375
private fun getObsoletedObjects(techName: String, ruleset: Ruleset, civInfo: Civilization?): Sequence<RulesetStatsObject> =
285376
(
286377
getFilteredBuildings(ruleset, civInfo) { true }
@@ -293,6 +384,7 @@ object TechnologyDescriptions {
293384
}
294385

295386
/** Readability - for the 'obsoleted' in [getTechEnabledIcons] */
387+
@Readonly @Suppress("purity") // only mutates the freshly-created icon it returns
296388
private fun RulesetStatsObject.getObsoletedIcon(techIconSize: Float) =
297389
when (this) {
298390
is Building -> ImageGetter.getConstructionPortrait(name, techIconSize)
@@ -306,6 +398,7 @@ object TechnologyDescriptions {
306398
}
307399

308400
/** Common filtering for both [getEnabledBuildings] and [getObsoletedObjects], difference via predicate parameter */
401+
@Readonly
309402
private fun getFilteredBuildings(
310403
ruleset: Ruleset,
311404
civInfo: Civilization?,
@@ -325,6 +418,7 @@ object TechnologyDescriptions {
325418
* nuclear weapons and religion settings, and without those expressly hidden from Civilopedia.
326419
*/
327420
// Used for Civilopedia, Alert and Picker, so if any of these decide to ignore the "Will not be displayed in Civilopedia"/HiddenFromCivilopedia unique this needs refactoring
421+
@Readonly
328422
private fun getEnabledUnits(techName: String, ruleset: Ruleset, civInfo: Civilization?): Sequence<BaseUnit> {
329423
return ruleset.units.values.asSequence()
330424
.filter {

‎core/src/com/unciv/ui/screens/pickerscreens/TechButton.kt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ import com.unciv.ui.components.extensions.toLabel
2121
class TechButton(
2222
techName: String,
2323
private val techManager: TechManager,
24-
isWorldScreen: Boolean = true
24+
isWorldScreen: Boolean = true,
25+
private val techIconsIndex: TechnologyDescriptions.TechIconsIndex? = null
2526
) : Table(BaseScreen.skin) {
2627

2728
internal val text = "".toLabel().apply {
@@ -114,7 +115,7 @@ class TechButton(
114115
val civ = techManager.civInfo
115116
val tech = civ.gameInfo.ruleset.technologies[techName]!!
116117

117-
TechnologyDescriptions.getTechEnabledIcons(tech, civ, techIconSize = 30f)
118+
TechnologyDescriptions.getTechEnabledIcons(tech, civ, techIconSize = 30f, iconsIndex = techIconsIndex)
118119
.take(5)
119120
.forEach { techEnabledIcons.add(it) }
120121

‎core/src/com/unciv/ui/screens/pickerscreens/TechPickerScreen.kt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import com.unciv.models.UncivSound
1717
import com.unciv.models.ruleset.tech.Technology
1818
import com.unciv.models.ruleset.unique.UniqueType
1919
import com.unciv.models.translations.tr
20+
import com.unciv.ui.objectdescriptions.TechnologyDescriptions
2021
import com.unciv.ui.components.NonTransformGroup
2122
import com.unciv.ui.components.extensions.colorFromRGB
2223
import com.unciv.ui.components.extensions.darken
@@ -146,6 +147,9 @@ class TechPickerScreen(
146147
for (label in eraLabels) label.remove()
147148
eraLabels.clear()
148149

150+
// Computed once for all techs instead of per-TechButton - see #15641
151+
val techIconsIndex = TechnologyDescriptions.buildTechIconsIndex(ruleset, civInfo)
152+
149153
val allTechs = ruleset.technologies.values
150154
if (allTechs.isEmpty()) return
151155
val columns = allTechs.maxOf { it.column!!.columnNumber } + 1
@@ -207,7 +211,7 @@ class TechPickerScreen(
207211
if (tech == null) {
208212
techTable.add(table).fill()
209213
} else {
210-
val techButton = TechButton(tech.name, civTech, false)
214+
val techButton = TechButton(tech.name, civTech, false, techIconsIndex)
211215
table.add(techButton)
212216
techNameToButton[tech.name] = techButton
213217
techButton.onClick { selectTechnology(tech, queue = false, center = false) }

0 commit comments

Comments
 (0)