CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.63k stars 4.17k forks source link

The jmapgen infrastructure uses a huge amount of RAM #50344

Open kevingranade opened 3 years ago

kevingranade commented 3 years ago

Describe the bug

When I was profiling the test exe memory usage (with heapcheck) to find the source of runaway allocation when the test suite was built with ASan, I discovered that the jmapgen infrastructure is responsible for a really huge amount of memory allocation. At the time of writing, it is consuming appproximately 380MB of RAM throughout the lifetime of the process. This isn't directly causing any bugs, but it seems like an excessive amount of allocation for what is happening, and it can definitely contribute to poor performance or even an inability to run the game on some computers. The allocation is occurring in mapgen_function_json_base::setup_common( const JsonObject &jo ) in mapgen.cpp Specifically, when placements are added to a mapgen function via objects.add( where, what );

Steps To Reproduce

Run the test exe or game exe under a heap profiler such as heapcheck or valgrind -tool=massif At startup, the process allocates ~380MB of memory in a vector containing objects of type std::pair<jmapgen_place, shared_ptr_fast<const jmapgen_piece> >.

Expected behavior

Less allocation, I wouldn't expect this particular data structure to exceed 50MB or so.

Screenshots

Screenshot from 2021-07-30 18-54-14 This is a heap profile of a cata_test run. The bottom-most orange rectangle is the allocation in question.

Versions and configuration

Additional context

The data structure in question consists of 3 jmapgen_int members and a shared_ptr. Each jmapgen_int is a pair of 32-bit integers, and the shared_ptr is 16 bytes. I tried reducing the definition of jmapgen_int to int16_t, which reduced the overll size of the allocations proportionally. I also added an invocation of vector::shrink_to_fit() after populating the vector. Togehether these two changes reduced the allocations to 181MB. While this is an improvement, this is still not the size of data structure I expect here. I think there is something systemically wrong happening here resulting in duplicate entries and bloating these vectors.

kaosushijin commented 3 years ago

I'm running a web-based implementation of C:DDA, leveraging ttyd, and memory constraints are my biggest issue. Trimming 300MB+ from the process would be incredible. Thumbs up and watching this one.

Zireael07 commented 3 years ago

@kaosushijin link please?

kaosushijin commented 3 years ago

@Zireael07 https://cdda.foxlight.info/

BrettDong commented 2 years ago

image

image

image

image

jbytheway commented 2 years ago

I did some back-of-the-envelope calculations to see if the current level of memory consumption is reasonable. Currently sizeof(std::pair<jmapgen_place, shared_ptr_fast<const jmapgen_piece> >) is 40 bytes. So, taking @BrettDong's calculation of ~150MB of RAM, suggests 3.75 million pairs.

On the other hand, we have 6165 mapgen objects in total (that includes some non-JSON ones, but I'm deeming those to be negligible), each 24x24 tiles. So that's ~3.5 million tiles. Slightly more than 1 pair per tile is perfectly reasonable.

So, if there is duplication it can only be because we have too many mapgen objects.

Querying with our json tools...

$ tools/json_tools/table.py --type=mapgen om_terrain | grep -v None | tr -d '|'\'',/[]' | xargs -n1 | sort -u | wc
   6109    6109  100888

It looks to me like roughly 6000 is a plausible number.

In conclusion, I think that there isn't anything surprising here. The only thing that's wrong is the pair being 40 bytes. We can certainly reduce that, and it sounds like Kevin already had done some work towards that.

kevingranade commented 6 months ago

I left this issue open because there is further potential to shrink jmapgen_place by another 100MB or so by shrinking it to something like the following, but it would be much more invasive and harm maintainability a bit.

struct jmapgen_place {
        int x_min : 6;
        int x_max : 6;
        int y_min : 6;
        int y_max : 6;
        int z_min : 5;
        int z_max : 5;
        int repeat_min : 14;
        int repeat_max : 14;
};