Openning New Game menu lags on PS Vita

[Districh-ru]

Preliminary checks

• I've checked that there aren't other open issues on the same topic.
• I've checked that this issue is reproducible on the latest snapshot build.

Platform

PlayStation Vita

Describe the bug

This issue is from Discord channel from Pønytail.
He has reported that in v. 1.1.3 and 1.1.4 there is about 40 seconds hang when opening the New Game menu and about 2.5 minutes lag to open Standard Game menu. It happens on the first time menu open and there is no lag until the game is restarted.
He has 1199 maps in maps folder.
In v. 1.1.2 and earlier it takes almost no time to open the New Game menu and about 20 seconds to open Standard Game menu.

The original issue description in Russian:
"Пока что из замеченного, при нажатии на "новую игру" приложение всё так же зависает примерно на 40 секунд, после чего нажатие на эту кнопку становится адекватным до следующего перезапуска приложения. Такое поведение тоже началось с версии 1.1.3."
"Всего у меня 1199 карт, на версии 1.1.2 отклик на нажатие кнопки "новая игра" в меню моментальная, при выборе "обычная игра" отклик примерно 20 секунд. На последних версиях отклик "новой игры" порядка 40 секунд, "обычной игры" 2.5 минуты. Т.е. скорость реакции приложения стала явно меньше. Заранее спасибо за работу!"

Save file

No save file, just put more than 1000 maps in maps folder and open the New Game and Custom Game menus.

Additional info

On Vita3K emulator with old CPU backend (Unicorn, without optimizations) and about 1500 maps it takes about 2 seconds to open Standard Game on v.1.1.2 and about 3 seconds on v.1.1.4, latest build. And on v.1.1.4 there is a noticeable lag (about 0.5 seconds) when opening New Game menu.

[oleg-derevenetz]

Hi @Districh-ru could you please ask someone with a real PS Vita device to test the #9310? Before the std::filesystem has been used instead of multiple home-grown I/O APIs, PSV didn't use the System::GetCaseInsensitivePath() logic (probably PSV in general has the case-insensitive FS), and this logic may be very costly in terms of I/O.

[ihhub]

I also noticed that we check the campaign scenario files in a very unoptimized way: for every scenario we read all files in a directory.

[oleg-derevenetz]

I also noticed that we check the campaign scenario files in a very unoptimized way: for every scenario we read all files in a directory.

Well, as far as I understand, this logic was there from the very beginning, so, although there is a room for improvements, it shouldn't be the cause of this regression, I think.

[Districh-ru]

I found a commit after which the menu "load" time increased by tests on Vita3K emulator: #9098
Here is a build with this PR merged (this build no commits applied after PR 9098): fheroes2_has9098.zip

And without this PR (previous commit): fheroes2_no9098.zip

Both builds estimate New Game menu load time and show it using in-game dialog.

[oleg-derevenetz]

Hi @Districh-ru

I found a commit after which the menu "load" time increased by tests on Vita3K emulator: #9098

Yes, that's why I introduced the #9310. I strongly doubt that it's due to migration std::filesystem, but that PR also enabled case-insensitive search for PS Vita, which may be redundant. What's about #9310?

[Districh-ru]

#9310 has the same time as the current build: 800 - 900 ms.
So the problem is in some other place
Pønytail reported the same:

попробовал запуск на всех трёх версиях - в версии "no" отклик составил 411 мс, в "has" 50 с, т.е. разница как-раз та, которую я наблюдаю с версии 1.1.3.

And about #9310:

В версии с оптимизацией отклик длительный, примерно кк в версии "has"

[Districh-ru]

@oleg-derevenetz, please check #9307 (comment)

[oleg-derevenetz]

Hi @ihhub

I also noticed that we check the campaign scenario files in a very unoptimized way: for every scenario we read all files in a directory.

Do you mean this function:

fheroes2/src/fheroes2/campaign/campaign_scenariodata.cpp

Lines 617 to 641 in 2b4f5e3

	bool tryGetMatchingFile( const std::string & fileName, std::string & matchingFilePath )
	{
	static const auto fileNameToPath = []() {
	std::map<std::string, std::string> result;
	const ListFiles files = Settings::FindFiles( "maps", "", false );
	for ( const std::string & file : files ) {
	result.try_emplace( StringLower( System::GetFileName( file ) ), file );
	}
	return result;
	}();
	const auto result = fileNameToPath.find( fileName );
	if ( result != fileNameToPath.end() ) {
	matchingFilePath = result->second;
	return true;
	}
	return false;
	}
	}

? If yes, then it's actually not that bad. The list of maps is read only once and cached (static const). This is much better than, say, performing a case-insensitive search in the file system every time you need to find a particular campaign map.

[ihhub]

Hi @oleg-derevenetz ,

I meant this code:

    bool CampaignData::isAllCampaignMapsPresent() const
    {
        for ( size_t i = 0; i < _scenarios.size(); ++i ) {
            if ( !_scenarios[i].isMapFilePresent() )
                return false;
        }

        return true;
    }

Having const ListFiles files = Settings::FindFiles( "maps", "", false ); call only once for this matter saves a lot of resources.

[oleg-derevenetz]

Hi @ihhub

I meant this code:

    bool CampaignData::isAllCampaignMapsPresent() const
    {
        for ( size_t i = 0; i < _scenarios.size(); ++i ) {
            if ( !_scenarios[i].isMapFilePresent() )
                return false;
        }

        return true;
    }

Having const ListFiles files = Settings::FindFiles( "maps", "", false ); call only once for this matter saves a lot of resources.

Yes, Campaign::ScenarioData::isMapFilePresent(), which is called here, in turn, calls the tryGetMatchingFile(), which maintains the static const cache, so no extra I/O should be performed.