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

Examining or opening advanced inventory on a tile with a lot of cooked foodstuffs causes big slowdown when it tries to stack items #76514

Closed CoroNaut closed 1 month ago

CoroNaut commented 1 month ago

Describe the bug

If you have like 200-300 already-cooked foodstuffs on a tile and examine or use advanced inventory on that tile to get the contents, there is a slowdown. This is due to it grabbing the crafting components of said food to check if it can stack everything together.

As you can see from the flame graph below, when I open advanced inventory and select the tile with cooked meat, most of the cpu time is spent on get_uncraft_components while it tries to stack each individual item.

Attach save file

TESTINGTWO-trimmed.tar.gz

Steps to reproduce

  1. Load save or alternatively craft a crap ton of food yourself
  2. examine the freezer with 500 cooked meats and notice a slowdown.
  3. Open advanced inventory manager and set a pane to look at that tile, notice a slowdown.

Expected behavior

Food should not save its crafting components like electronics, clothing, armor, vehicle parts, etc. It is food. Once it's cooked or transformed, it can't be disassembled, and this is enforced in-game. It is completely unused information from the games' perspective, saved and forgotten.

The only use for it is the Made from section for food. I definitely don't think this is worth keeping around when it causes this much slowdown. I'm sure we could go without this tidbit of information as we play the game.

My proposal is to remove the crafting ingredients from food. This will make the stacking code much faster, and will barely affect the user experience, except for increased performance.

Screenshots

opening fridge with cooked items3 Untitled

Versions and configuration

Additional context

No response

GuardianDll commented 1 month ago

prooly related to #73502

CoroNaut commented 1 month ago

Did a quick read over that issue, it seems very similar, if you want to mark mine as a duplicate, go for it.

akrieger commented 1 month ago

ugh I love when people report performance issues with flame graphs.

30 seconds of searching got me to this gem

requirement_data requirement_data::disassembly_requirements() const
{
    // TODO:
    // Allow jsonizing those tool replacements

    // Make a copy
    // Maybe TODO: Cache it somewhere and return a reference instead

From quick looking around, it should be possible to make this almost-zero-copy instead of having to create and destroy a ton of ephemeral items.

akrieger commented 1 month ago

Ok here's where I'm confused though. I don't actually see where in get_uncraft_components we actually create temporary items. I see a lot of item_comp's getting created though. Time to hit a debugger it seems.

akrieger commented 1 month ago

Oh gosh, it's so silly. One character fix.