Very slow "Use item" menu

[RDru]

Describe the bug

Pressing 'a' for use item menu in attached save takes ~7 seconds for menu to open.

I have managed to locate what is causing the slowness.

iuse_actor.cpp:
ret_val<bool> iuse_transform::can_use( const Character &p, const item &, bool,
                                       const tripoint & ) const

line 261 currently
adding simple return on the beginning of that method makes the action almost instant

I don't know what it does exactly but I think it gathers and checks whole new inventory from all items around. And it does it for all items(?).

Steps To Reproduce

Steps to reproduce the behavior:

1. Load attached save on build 9853 or later (game is just saved on that version,)
2. Press 'a';

Expected behavior

It should take far less time to open that menu.

Versions and configuration

Additional context

save

[ZhilkinSerg]

That's quite a lot of items.

[wapcaplet]

The attached save link is no longer available, but I can still reproduce this in 0.E-6107-g 973dd89

• Use the debug menu to spawn, w item, Everything, (quantity 1).
• Try the Eat menu - it comes up almost instantly, though it's ~12 pages long
• Try the activate menu - it takes 15-20 seconds to generate the ~21 pages

[Aivean]

I just took a quick look at it and something looks suspicious:
Character::can_pickVolume(item const&, bool) is called five times in different places (I assume for the same items).

Can somebody who's more familiar with item management check that out?

[Aivean]

So I tried adding a simple cache to pickup_inventory_preset::get_denial:

mutable std::unordered_map<const item*, std::string> denial_cache;

And it speeds up activation menu opening at least by half (1.1 seconds -> 0.5 seconds).

Before

After

@KorGgenT, can you evaluate this idea? I'm not sure about the lifetime of pickup_inventory_preset and whether using a cache there won't break something.

[kevingranade]

I tried spawning 10 of every item and opening the use item menu, it opens instantaneously. Did we have some major improvements to this or am I attempting to reproduce the issue incorrectly?

EDIT: Ok so I went overboard and did "spawn every item" 9 times in a grid so I would be surrounded by items in all directions, and I definitely can see some slowdown now, though it's a fraction of a second.
The profile looks the same as reported earlier, so it seems unlikely this was addressed and I'm just on a relatively fast system now.

Now that I'm code diving, doing "one of every item" instead of "10 of every item" is what made the difference, because it's only looking at adjacent tiles anyway.

[kevingranade]

@Aivean the lifetime of pickup_inventory_preset (or any of the inventiry_preset subclasses) is a single invocation of game_menus::inv::use(), so yes accumulating in a simple map should be safe.

[kevingranade]

Other than Aivean's proposal, this needs pervasive nested item caching, which isn't a reasonable thing to try and shim in before 0,F