#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 )
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 Grid
s which does the following:
- Create a new array
- Allocate memory for it and copy the data from
m_PassabilityMap
- Set the
data
property ofm_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)
by , 8 years ago
Attachment: | 0001-Remove-unused-function-for-the-AI-connectivity-grid.patch added |
---|
comment:1 by , 8 years ago
by , 8 years ago
Attachment: | 0003-Make-AI-map-updates-more-clever.patch added |
---|
comment:3 by , 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 , 8 years ago
I am quite puzzled by this new stack. Was that a "not enough memory" problem you experienced?
comment:5 by , 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.
follow-up: 7 comment:6 by , 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 , 8 years ago
Attachment: | AIWorker-update-rewrite.patch added |
---|
comment:7 by , 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 , 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 , 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 =)
follow-up: 11 comment:10 by , 8 years ago
Please, can somebody summarize, whether AIs can expect more or less information from this patch?
follow-up: 12 comment:11 by , 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.
comment:12 by , 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:15 by , 8 years ago
Keywords: | review removed |
---|
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. :)