Remove useless cache and serialization of technologies TechnologyManager
ClosedPublic
Actions

Authored by elexis on Dec 8 2017, 5:17 PM.

Details

Reviewers

mimo

Commits

rP20610: Remove unused serialized cache of technology templates in TechnologyManager.

Trac Tickets

#4239
#3834

Summary

The TechnologyManager currently caches and serializes technologies that had been researched.
However the templates are never used, so looking up and caching the templates is wasted performance.
Serializing the JSON templates might make it easier to debug OOS issues, but we decided to minimize the data serialized (#3834).
Both AI and simulation have their ways to get the technology templates if they want it.
(The removal of the serialization will allow unification of tech getters in globalscripts, D1108).

See linked trac tickets and D1108.

Test Plan

Notice that the AI still works and that all occurrences of the function are covered.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Dec 8 2017, 5:17 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/TechnologyManager.js
|  66| »   »   »   ||·(tech.top·&&·(this.IsTechnologyResearched(tech.top)·||·this.IsTechnologyResearched(tech.bottom))))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/TechnologyManager.js
| 232| »   »   »   for·(var·component·in·modifiedComponents)
|    | [NORMAL] JSHintBear:
|    | 'component' is already defined.

binaries/data/mods/public/simulation/components/TechnologyManager.js
| 238| »   »   var·cmpTemplateManager·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_TemplateManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpTemplateManager' is already defined.

binaries/data/mods/public/simulation/components/TechnologyManager.js
| 239| »   »   var·template·=·cmpTemplateManager.GetCurrentTemplateName(msg.entity);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/TechnologyManager.js
| 244| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(msg.entity,·IID_Identity);
|    | [NORMAL] JSHintBear:
|    | 'cmpIdentity' is already defined.

binaries/data/mods/public/simulation/components/TechnologyManager.js
| 247| »   »   »   »   var·classes·=·cmpIdentity.GetClassesList();
|    | [NORMAL] JSHintBear:
|    | 'classes' is already defined.

Harbormaster completed remote builds in B4016: Diff 4651.Dec 8 2017, 5:21 PM

nice catch with the useless caching
I wonder if researchQueued should not become a Map now (by analogy with the other two, and deleting properties of an object is usually not recommended even if performance does not matter for that one).

This revision is now accepted and ready to land.Dec 8 2017, 7:00 PM

Closed by commit rP20610: Remove unused serialized cache of technology templates in TechnologyManager. (authored by elexis). · Explain WhyDec 8 2017, 7:35 PM

This revision was automatically updated to reflect the committed changes.

Owners added a subscriber: Restricted Owners Package.Dec 8 2017, 7:35 PM

Even uselessly copied through the GUIInterface each turn.
Thanks for the reviews mimo!
I fully agree with using the Map, gives a much better symmetry.

elexis mentioned this in rP20737: Replace DataTemplateManager simulation component with a globalscript, refs….Dec 31 2017, 2:03 AM