CleverRaven / Cataclysm-DDA

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

Performance regression in "Items" step of the Verifying process in loading a game #71371

Open NetSysFire opened 8 months ago

NetSysFire commented 8 months ago

Describe the bug

Compared to some months ago (around November), this step takes significantly longer and I can not find the cause for it.

Attach save file

n/a

Steps to reproduce

  1. Load or create any game.
  2. Notice that said step is taking very long.

image

Expected behavior

It does not take that long.

On the devcord a rando confirmed that they noticed the same, but its more of an anecdote than anything, so I will not be marking this as confirmed on creation. I also tried to "bisect" via using older releases but for the love of $deity I can not really find known good and bad candidates except a vague "one from November somewhere" because I did not record which build I was using prior to updating.

I did crudely measure the time it takes to load a game with /usr/bin/time (just exited as soon as loading was done):

11.39s user 0.54s system 80% cpu 14.853 total

vs

14.90s user 0.67s system 86% cpu 18.073 total

But it feels significantly longer than that.

Screenshots

No response

Versions and configuration

Additional context

No response

ZhilkinSerg commented 8 months ago

image

ZhilkinSerg commented 8 months ago

Item consistency just spawns a lot of items, so it adds up to a significant time. This check can be moved to a test suite or hidden behind some option to speed up loading times or maybe check results can be cached somehow.

RenechCDDA commented 8 months ago

@akrieger bisected this last night and surprisingly the major culprit appears to have been a json change. Link to discussion: https://discord.com/channels/598523535169945603/598535827169083403/1202085328475922432

"the first major regression from 9ish seconds to 22/25ish seconds was #70370"

image

(a follow-up this morning, the day after): "if i do this on master then item verification drops to only about 6 seconds"

turco-ai commented 8 months ago

I can confirm that the game would get stuck on items for quite a while and I thought that was normal because more stuff got added, I'm on Android so I feel every performance improvement.(and deprovements if you will)

Inglonias commented 7 months ago

@akrieger bisected this last night and surprisingly the major culprit appears to have been a json change. Link to discussion: https://discord.com/channels/598523535169945603/598535827169083403/1202085328475922432

I wonder if this is happening because of the sheer size of the item stacks being counted here? That's all I can think of, anyway.

SurFlurer commented 7 months ago

diff --git a/data/json/itemgroups/supplies.json b/data/json/itemgroups/supplies.json
index 151eec6439..5b6909769d 100644
--- a/data/json/itemgroups/supplies.json
+++ b/data/json/itemgroups/supplies.json
@@ -56,25 +56,25 @@
     "id": "sandbag",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_sand", "count": 2500 } ]
+    "items": [ { "item": "material_sand", "charges": 2500 } ]
   },
   {
     "id": "sandbag_part",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_sand", "count": [ 400, 2500 ] } ]
+    "items": [ { "item": "material_sand", "charges": [ 400, 2500 ] } ]
   },
   {
     "id": "gravelbag",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_gravel", "count": 2500 } ]
+    "items": [ { "item": "material_gravel", "charges": 2500 } ]
   },
   {
     "id": "gravelbag_part",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_gravel", "count": [ 400, 2500 ] } ]
+    "items": [ { "item": "material_gravel", "charges": [ 400, 2500 ] } ]
   },
   {
     "id": "earthbag",
@@ -104,25 +104,25 @@
     "id": "cement_bag",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_cement", "count": 2800 } ]
+    "items": [ { "item": "material_cement", "charges": 2800 } ]
   },
   {
     "id": "cement_bag_part",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_cement", "count": [ 400, 2800 ] } ]
+    "items": [ { "item": "material_cement", "charges": [ 400, 2800 ] } ]
   },
   {
     "id": "quicklime_bag",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_quicklime", "count": 1200 } ]
+    "items": [ { "item": "material_quicklime", "charges": 1200 } ]
   },
   {
     "id": "quicklime_bag_part",
     "type": "item_group",
     "container-item": "bag_durasack",
-    "items": [ { "item": "material_quicklime", "count": [ 400, 1200 ] } ]
+    "items": [ { "item": "material_quicklime", "charges": [ 400, 1200 ] } ]
   },
   {
     "id": "paintcans",

Although using charges, this change does cut the time taken by"Items" step by 60%.

Zireael07 commented 7 months ago

I think this is the confirmation that the move to get rid of charges will bring about perf trouble

NetSysFire commented 5 months ago

I am afraid this is happening again since a week or two or so. Edit: This should not affect the 0.H release since the cause will probably not be backported.

NetSysFire commented 2 months ago

perf.data.gz

Due to using my own build, I managed to gather perf data since I had debugging symbols there.

Maleclypse commented 2 months ago

You’ll want to move it back to potential blockers.

Edit: i remembered where the button was on mobile.