CleverRaven / CleverRaven/Cataclysm-DDA

The jmapgen infrastructure uses a huge amount of RAM

Open
#50,344 6 comments 4 reactions 0 assignees View on GitHub
[C++] <Bug> Code: Performance Map / Mapgen
Dominant language
C++
Stars
13.2k
Forks
4.6k
Avg merge
2d 1h
Merged PRs (30d)
205

Description

### 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 >`.

### 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](https://user-images.githubusercontent.com/860276/127725233-86b969c5-dfac-4852-98d2-b5481fb78c01.png)
This is a heap profile of a cata_test run. The bottom-most orange rectangle is the allocation in question.

### Versions and configuration

- OS: Ubuntu 20.04
- Game Version: cdda-experimental-2021-07-30-2347-4-gf4e7fe869e
- Graphics version: Terminal
- Ingame language: US
- Mods loaded: dda

### 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.

Contributor guide

Open the contributing guide

Research direction

Start in mapgen.cpp at mapgen_function_json_base::setup_common(const JsonObject &jo), focusing on placements added through objects.add(where, what). Reproduce the allocation with the test or game executable under heapcheck or Valgrind Massif, then inspect the vector of jmapgen_place/shared_ptr pairs for duplicate entries or other sources of bloat. Done means identifying and addressing the systemic allocation problem while keeping usage near the reported expected size.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
game-dev, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.