short string info generates too long (725 ms)

[Brambor]

Describe the bug

Bringing up this short string info takes a lot of time (every time):

I suspect it is because of all the things it can be crafted into.

TODO: profile it, add (? miliseconds) to title.

Attach save file

TEST Winter Beach-trimmed.tar.gz

Steps to reproduce

1. Load save
2. open / AIM OR ( disassembly menu
3. e examine short string
4. Take note of how long it took

Expected behavior

Cache it or something. Something could be: don't waste too many resourses on the , and 683 more.

Screenshots

No response

Versions and configuration

• OS: Windows
  • OS Version: 10.0.19045.5247 (22H2)
• Game Version: 0.G-15789-g5ee77a788a [64-bit] - master was e4f5f67
• Graphics Version: Tiles
• Game Language: English [en]
• Mods loaded: [
  Dark Days Ahead [dda],
  Disable NPC Needs [no_npc_food],
  Portal Storms Ignore NPCs [personal_portal_storms],
  Slowdown Fungal Growth [no_fungal_growth]
  ]

Additional context

No response

[ZhilkinSerg]

[Brambor]

Thanks! If nothing else, crafting_inventory() and get_group_available_recipes() should only be called once, if it isn't already.

[PatrikLundell]

Crafting_inventory() is called once per item, i.e. it's built before the loop checking all the items that could potentially be constructed from it, as is get_group_available_recipes(). (This is the code marked in the image @ZhilkinSerg posted above).

What could potentially be done would be to cache these at the start of the UI code so it would be available to each item you examine as you move through the UI. However, that would mean the item::final_info operation would have to access this cached info rather than building the info, and so each call to it would have to build the cache beforehand as well (and you'd have to make sure future potential users of the operation would know about the required cache preparation).

However, item::final_info seems to be used only by item::get_info(vector of iteminfo version), but there's a splitting usage tree above that.
MSVC didn't like my attempt to expand the Check Call Hierarchy, so that's how far I got before it hung.

[Brambor]

Thanks! I would like to look into implementing it, if you haven't already. Reading your analysis I believe it will be pretty easy to do.

• Note: Cache item info for crafting GUI full-text search #77914 This can probably be used. It would be very nice, if it just shares this cache!

[PatrikLundell]

Please feel free to run with it!

[Brambor]

Wrong analysis

Our analysis differs:

Analysis of info from short string on save TEST Winter Beach-trimmed.tar.gz. Open AIM examine short string.

I wonder if my MSVS 2019 does construct the crafting_inv and available_recipe_subset on the line const bool can_make =:

        const itype_id &tid = typeId();
// move this to...
        const inventory &crafting_inv = player_character.crafting_inventory();
// move this to...
        const recipe_subset available_recipe_subset = player_character.get_group_available_recipes();
        const std::set<const recipe *> &item_recipes = available_recipe_subset.of_component( tid );

        insert_separation_line( info );
        if( item_recipes.empty() ) {
            info.emplace_back( "DESCRIPTION", _( "You know of nothing you could craft with it." ) );
        } else {
            std::vector<std::string> crafts;
            for( const recipe *r : item_recipes ) {
// ... here
                const bool can_make = r->deduped_requirements()
                                      .can_make_with_inventory( crafting_inv, r->get_component_filter() );

                // Endarken recipes that can't be constructed with the survivor's inventory
                const std::string name = r->result_name( /* decorated = */ true );
                crafts.emplace_back( can_make ? name : string_format( "<dark>%s</dark>", name ) );
            }
            const std::string recipes = enumerate_lcsorted_with_limit( crafts, 15 );
            info.emplace_back( " DESCRIPTION", string_format( _( "You could use it to craft: %s" ), recipes ) );
        }

And thus make the analysis less useful. If there is a setting in MSVC for that @ZhilkinSerg, please let me know! I want to run it with the default "Release" option making it -O2, because that gives the results the user will see.

Right now, I will cache crafting_inv and available_recipe_subset. If your analysis is correct, time should drop by 66%. But having MSVC tell me that directly would be much more useful in the future!

Never mind. The two values are cached, you provided analysis for the uncached case, and I was analyzing the cached case.