Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#4191 closed defect (fixed)

[PATCH] Clever map updates for the AI

Reported by: Itms Owned by: Itms
Priority: Release Blocker Milestone: Alpha 21
Component: Core engine Keywords: patch
Cc: Stan Patch:

Description (last modified by Itms)

The underlying issue for the memory crash everybody experiences is the update of the passability map for the AI. It uses our custom ToJSVal for Grids which does the following:

  • Create a new array
  • Allocate memory for it and copy the data from m_PassabilityMap
  • Set the data property of m_PassabilityMapVal to the new array
  • (Later the old value of data will be deleted by the GC since it is not used anymore)

Thus, in the specific case of the AI map, this is sub-optimal and causes the allocation of a large chunk of memory (the entire passability map, array of size width*height*size(u16)) for nothing. This patch adds a custom function that modifies the already allocated passability data. It also uses that method for the territory map, which is allocated the same way but causes less problems since it is far smaller. Additionally, I propose to remove the function GetConnectivityGrid which is not used by the AI and has the same flaw. If the AI needs such a grid in the future, it should work the same way as the passability and territory ones. This fixes the memory crash I experience in large maps (like Stan's new Corinthian Isthmus(4)), but other people should also test it.

Attachments (3)

Change History (19)

comment:1 by Itms, 8 years ago

Updated version after comments on IRC.

Also I tested something I had forgotten: this patch does not change the simulation state over a large game. :)

comment:2 by Itms, 8 years ago

In 18711:

Add a linker flag on Windows allowing the game to be large address aware. This allows the game to use more than 2GB of RAM. https://msdn.microsoft.com/en-us/library/wz223b1z.aspx

However, this can mask memory issues, so this flag will not be enabled by default. The autobuilder will use it, so SVN playtesters and release users can benefit from it. Windows users compiling the game themselves will have the 2GB limitation.

Fixes #4190, refs #1619, #4191
Flag tested by Stan

comment:3 by Stan, 8 years ago

Hey Itms, sorry for taking so much time to test it. I can confirm this slows down the memory increase. It does not completely stop it though

Things to take into account:

  • I am using Corinthian Ismuth (4)
  • I am using x2 speed
  • I had not SM38 when I reported the issue
  • I was running into a debugger the release build, maybe that changes some stuff
Time R.A.M used
12 min 1 326 MB
15 min 1 386 MB
18 min 1 419 MB
21 min 1 450 MB
25 min 1 505 MB
28 min 1 570 MB
31 min 1 613 MB
32 min 1 635 MB

The game crashed at 32:19 min. This is significantly better than the 2:55 min I was used to. I guess that should solve most issues people are experiencing.

Call Stack : http://pastebin.com/PWCvFJCH

comment:4 by Itms, 8 years ago

I am quite puzzled by this new stack. Was that a "not enough memory" problem you experienced?

comment:5 by Stan, 8 years ago

It's like it's not constantly reproductible, I managed to get the same call stack, and then clicked continue. That gave me that call stack : http://pastebin.com/YF0801t3

Don't know if it's more logical

Exception thrown at 0x76AB96C2 in pyrogenesis.exe: Microsoft C++ exception: std::bad_alloc at memory location 0x007EED2C.
Unhandled exception at 0x76AB96C2 in pyrogenesis.exe: Microsoft C++ exception: std::bad_alloc at memory location 0x007EED2C.

pyrogenesis.exe has triggered a breakpoint.

Exception thrown at 0x0F54C7EC (msvcr120.dll) in pyrogenesis.exe: 0xC0000005: Access violation reading location 0x0000006C.

Time of crash 34:21 min

I managed to have a 58min game before that with no crash so i don't know what's happening.

comment:6 by Itms, 8 years ago

Here is a bigger, nicer, cleverer patch. I split the big StartComputation function into smaller chunks where actual checks can be performed. I got rid of several dummy allocations and memory footwork.

Stan, does that improve the situation on your end?

by Itms, 8 years ago

in reply to:  6 comment:7 by Stan, 8 years ago

Replying to Itms:

Stan, does that improve the situation on your end?

It seems so. I didn't manage to make it crash aside from alt-tabbing too much >< so I guess it's more stable now. Memory didn't get higher than 1.32GB and immediatly goes down when AI wins from that point to 700-900MB, I noticed that it doesn't affect the lag at the end of the game though. I had like 3fps after 1hour play even though two players were defeated. Guess there were still too many units moving.

comment:8 by Itms, 8 years ago

Great, thanks for the feedback. :)

The patch only changes how the memory is managed, not how the game computes things, so it's perfectly normal to still have the usual lag.

comment:9 by Stan, 8 years ago

Yeah it makes sense.

I don't know why AI is so much memory intensive while the pathfinder is CPU intensive.

So this patch plus the Large adress one should fix most issues with memory for now =)

comment:10 by agentx, 8 years ago

Please, can somebody summarize, whether AIs can expect more or less information from this patch?

in reply to:  10 ; comment:11 by Itms, 8 years ago

Replying to agentx:

Please, can somebody summarize, whether AIs can expect more or less information from this patch?

The data passed to the AI is exactly the same with the patch. Only memory management in the engine is touched.

in reply to:  11 comment:12 by Itms, 8 years ago

Replying to Itms:

Replying to agentx:

Please, can somebody summarize, whether AIs can expect more or less information from this patch?

The data passed to the AI is exactly the same with the patch. Only memory management in the engine is touched.

Sorry I forgot something - I am talking about the second patch (AIWorker-update-rewrite). The first patch (Remove-unused-function-for-the-AI-connectivity-grid) removes a function that Petra never uses, but if you find it useful I can leave it.

comment:13 by Itms, 8 years ago

Resolution: fixed
Status: newclosed

In 18759:

More clever map updates for the AI.

This patch fixes frequent memory issues by removing several dummy allocations and a lot of memory footwork.

Fixes #4191

comment:14 by Itms, 8 years ago

In 18760:

Remove a function relying on systems that are not optimized at all. This function is not actually used by Petra.

If it is needed at some point, this map should be handled and updated the same way as the passability map and territory map are.

Refs #4191

comment:15 by Itms, 8 years ago

Keywords: review removed

comment:16 by Itms, 8 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.