Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#3060 closed defect (fixed)

[PATCH] Improve AI manager deserialization efficiency

Reported by: historic_bruno Owned by:
Priority: Should Have Milestone: Alpha 19
Component: Core engine Keywords: patch
Cc: Patch:

Description

Currently, the AI manager component takes a relatively long time to deserialize (over 1 second in my testing on the Trading Demo map, compared to << 10 ms for other components). This isn't as noticeable when rejoining multiplayer games or loading saved games, but it makes the serialization test mode unusable. In fact, it's true whether AIs are in the game or not, so I think at least it should avoid serializing all this data if there are no AI players.

TemplateManager TIMER| : 1.36027 ms
AIManager TIMER| : 1.16718 s
CommandQueue TIMER| : 30.1005 us
ObstructionManager TIMER| : 252.203 us
ParticleManager TIMER| : 8.38414 us
Pathfinder TIMER| : 876.219 us
ProjectileManager TIMER| : 7.41657 us
RangeManager TIMER| : 6.75018 ms
SoundManager TIMER| : 13.3694 us
Terrain TIMER| : 13.5852 us
TerritoryManager TIMER| : 366.301 us
UnitRenderer TIMER| : 7.93691 us
WaterManager TIMER| : 6.14063 us
AIInterface TIMER| : 3.45523 ms
AuraManager TIMER| : 28.1265 us
Barter TIMER| : 25.7129 us
EndGameManager TIMER| : 29.6226 us
GuiInterface TIMER| : 42.7603 us
PlayerManager TIMER| : 24.9956 us
TechnologyTemplateManager TIMER| : 8.11913 ms
Timer TIMER| : 193.305 us
Trigger TIMER| : 49.7951 us
ValueModificationManager TIMER| : 31.7276 us
Decay TIMER| : 11.8056 ms
Footprint TIMER| : 173.553 us
Minimap TIMER| : 96.8564 us
Obstruction TIMER| : 271.925 us
OverlayRenderer TIMER| : 83.98 us
Ownership TIMER| : 124.728 us
Position TIMER| : 128.508 us
RallyPointRenderer TIMER| : 1.09329 ms
Selectable TIMER| : 424.19 us
TerritoryInfluence TIMER| : 184.045 us
UnitMotion TIMER| : 198.69 us
Vision TIMER| : 210.534 us
VisualActor TIMER| : 385.642 us
AIProxy TIMER| : 698.281 us
AlertRaiser TIMER| : 228.098 us
Armour TIMER| : 431.92 us
Attack TIMER| : 155.708 us
AttackDetection TIMER| : 410.923 us
BattleDetection TIMER| : 147.5 us
BuildRestrictions TIMER| : 318.869 us
BuildingAI TIMER| : 415.117 us
Cost TIMER| : 490.635 us
EntityLimits TIMER| : 526.097 us
Fogging TIMER| : 521.868 us
GarrisonHolder TIMER| : 504.523 us
Guard TIMER| : 503.77 us
Health TIMER| : 858.014 us
Identity TIMER| : 1.11391 ms
Loot TIMER| : 502.835 us
Looter TIMER| : 169.285 us
Player TIMER| : 1.88843 ms
ProductionQueue TIMER| : 888.246 us
RallyPoint TIMER| : 276.658 us
ResourceDropsite TIMER| : 167.851 us
ResourceGatherer TIMER| : 418.388 us
Sound TIMER| : 758.787 us
StatisticsTracker TIMER| : 665.636 us
StatusBars TIMER| : 456.078 us
TechnologyManager TIMER| : 1.04691 ms
TerritoryDecay TIMER| : 302.879 us
Trader TIMER| : 451.174 us
UnitAI TIMER| : 1.0146 ms
Visibility TIMER| : 470.667 us

Attachments (2)

aimanager.diff (2.5 KB) - added by mimo 4 years ago.
aimanager2.diff (2.5 KB) - added by leper 4 years ago.
Somewhat nicer version.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 5 years ago by mimo

I agree that the AI deserialization takes some time (the AI temporarily serializes all entities, waiting for #1089 to be fixed), but it should not be run when no AIs are in the game. Are you sure you had no AIs in your test ? from the IRC logs, I understand that you were testing on the "trading demo" map which by default has AI players (I don't know why, would be better without). Have you unassigned them when testing ?

comment:2 Changed 5 years ago by historic_bruno

Yeah, I had them unassigned anyway because the serialization test would instantly fail with AIs (due to #1089).

comment:3 Changed 5 years ago by mimo

I had a look at this, and in fact, most of the time is spent loading the templates in CCmpAIManager. Of course, for games without AI, we could avoid this step, but in the future, we want to be able to do serialization tests also with AI and in addition it would be nice to be able to replace on the fly a quitting player by an AI such that even games without AIs should be able to deal with AIs

So I guess we have to decide on the design of the AI serialization. I see two big components here which are the entities and the templates. We can either transfer them from simulation when deserializing (this is much cleaner, but really inefficient when doing serialization tests), or let the AI serializes its own copy (that's a bit of data dupplication, but would be much faster for serialization tests).

The present state is a bit hybrid. Because of #1089, the transfer from simulation of entities when deserializing does not work (that would still slow down ben's tests even more), but still the loading of templates is running. And the AI has its own copy of entities and templates, which was not serialized in ben's test as there was no AI : this was supposed to be temporary waiting for #1089, but it is maybe a better long-term solution in term of performances ?

comment:4 Changed 4 years ago by leper

Do you have the timer diff still somewhere?

comment:5 Changed 4 years ago by sanderd17

Note that since r16473 the AIInterface has a .Disable() function that disables data gathering there.

The AIInterface was no problem on game start (it starts clean), but it kept gathering a big number of messages send when there was no AI to flush them. So resulting in increasingly large objects as the game continues. This made OOS logs a pain to open in a text editor (which was my main reason to solve this).

Maybe the .Disable() function could call a .Disable() on the AIManager too? Or perhaps the other way around.

comment:6 Changed 4 years ago by mimo

I've tested with the profiler in replay mode, on the demo trade map, without AI and doing nothing on a few turns. Without serialization, I got less than 1 ms/turn for the full sim update. With serializationTest, this was about 700 ms With SerializationTest?, but commenting the template loading in CCmpAIManager, this was about 300 ms.

Futhermore, this loading is done for nothing as the AI uses its own serialized copy of the templates (as described in my comment 3). So it can be commented to speed up serialization tests.

But all these are just temporary hacks, and I think we should agree on the design of the AI serialization. I think we have two possibilities:

  • we may either load templates and entities at deserialization (as it is supposed to be done now). This is the cleaner, but would be a major performance problem if we want to be able to do any serialization tests with AI in the future. And when no AIs, we should then disable the AIManager.
  • or we may let the shared AI serialize itself entities and templates (which it is doing currently, waiting for a fix of #1089). That would be a bit of data dupplication, but much faster in term of performances for serialization tests.

Any inputs on that ?

comment:7 Changed 4 years ago by leper

Do you have an idea how long loading the template data per AI takes currently?

If that is a lot less than those 300ms I'm inclined to go with the second option as that shouldn't make it a lot harder for AI writers to do serialization as it is done in the shared AI.

comment:8 Changed 4 years ago by mimo

Well, I naively thought it would be much smaller, but it happens to be about 900 ms (to serialize and then deserialize all the templates in the shared AI, I don't know which one is the biggest). So we should stick to the first option and disable the AIManager when no Ais.

comment:9 Changed 4 years ago by mimo

In 16626:

fix entities deserialization in AIInterface and remove duplicates entities and templates in sharedAI, refs #3060 and fixes #1089

Changed 4 years ago by mimo

Attachment: aimanager.diff added

comment:10 Changed 4 years ago by mimo

Keywords: patch review added
Summary: Improve AI manager deserialization efficiency[PATCH] Improve AI manager deserialization efficiency

I've attached a possible fix to prevent the loading of templates when no Ais. In my tests, doing a serializationtest without Ais in the trade map goes from 650 ms to 280 ms after the patch.

Changed 4 years ago by leper

Attachment: aimanager2.diff added

Somewhat nicer version.

comment:11 Changed 4 years ago by leper

That file still uses a mix of unitN_t and uN, though the latter appears more often, so maybe we could switch the whole thing to use one or the other.

comment:12 Changed 4 years ago by mimo

Milestone: BacklogAlpha 19
Resolution: fixed
Status: newclosed

Thanks leper for the improved patch. Ticket fixed by r16633

Last edited 4 years ago by mimo (previous) (diff)

comment:13 Changed 4 years ago by mimo

Keywords: review removed
Note: See TracTickets for help on using tickets.