peter-kish / gloot

A universal inventory system for the Godot game engine.
MIT License
625 stars 28 forks source link

Performance impact sort() and merging inventories. v2.4.9 #211

Open TreaBeardGaming opened 5 months ago

TreaBeardGaming commented 5 months ago

There is a performance impact on .autosplitmerge() and .sort() v2.4.9

Scenario would be adding multiple items per second to inventory.

Using autosplitmerge() from one inventory to another:

  1. Plenty of space in inventory no impact
  2. Space for some items like 1x1 but not 1x2 or 2x2 etc, minimal impact fps begins to drop
  3. No space for new items, must fill existing stacks, large performance impact, clears up after a second or two.
  4. Completely full inventory and multiple items and trying to be added, large impact ui begins to freeze and FPS tanks to 0 if actions continue the application becomes unresponsive.

Using .sort() on an inventory using autosplitmerge and sorting after every item is merged:

  1. Minimal impact while inventory is empty or partially filled.
  2. Large impact 2-3 seconds per action when inventory is full or has no slots for new items

Using automerge() from one inventory to another:

  1. Minimal impact to FPS as it does drop from 60FPS to 30-35FPS but it stabilizes during high interactions at that FPS and returns to normal afterwards

Using .sort() on an inventory using automerge and sorting after every item is merged:

  1. Minimal impact if at all anything noticable while there are slots open and space for items to merge.
  2. Large impact to FPS when no slots are available or limited slots open, FPS tanks to 0FPS if actions persist

TLDR in my testing:

Using .sort() after every item addition is probably bad move for your game project, don't do it. Using automerge works perfectly fine in low volume, try not to add 20 items a second if it can be avoided. Using autosplitmerge works great as it merges items and fills any stacks that are not full, but has a huge performance impact so use sparingly.

peter-kish commented 5 months ago

Hey, thanks for the detailed analysis! I started using quadtrees for organizing items in 2d space (i.e. on a grid) which fixed some of the issues from earlier, but apparently comes with a performance cost. I think I'll put together a benchmarking scene and try to figure out which parts of automerge and autosplitmerge can be optimized further.

peter-kish commented 4 months ago

I did a few optimizations and bugfixes in v2.4.9 but I'm afraid that it only improves performance to some degree. The optimizations mostly affect the "automerge" and "autosplitmerge" calls, while i don't think there's much I can do regarding sort() since it's generally an expensive operation. But maybe making only once sort() call after all items have been added could be an acceptable workaround in your case? On the bright side, the issue where the application gets totally stuck should be fixed now 🙂.

I'll keep this issue open as I don't think the work is really done here (as it never is with optimizations). I'll try to come up with even more tricks to make the grid-based operations faster and report the progress here.