ladislav-zezula / StormLib

Official GitHub repository of the StormLib library created by Ladislav Zezula (author)
http://www.zezula.net/mpq.html
MIT License
561 stars 214 forks source link

SListFileCreateNodeForAllLocales Performance regression #269

Closed m4xw closed 1 year ago

m4xw commented 1 year ago

Offending commit: https://github.com/ladislav-zezula/StormLib/commit/503ab19d3d4253fb013752572c19c314d19de792

Description: Loading times regressed, on normal start for our projects use-case it will rarely find a entry at first so it iterates over the whole hash list.

Adding a break reduces the overhead by ~30% for cases where it finds a entry (just to put it into perspective) Adding the old code-path with GetFirstHashEntry back (which returns NULL for most invocations at first i guess), reduces overhead by 520%, reducing startup time on some embedded devices from 26sec to 5sec total.

No obvious regression was observed, so the suggestion is to partially revert the change in SListFileCreateNodeForAllLocales

ladislav-zezula commented 1 year ago

I'll need to see the MPQ in question to see where the overhead is.

m4xw commented 1 year ago

I cant provide the full mpq due to legal reasons (game assets), I could give u the steps to generate it yourself or provide it with zero'd contents potentially.

ladislav-zezula commented 1 year ago

Well, I cannot revert the commit, because it is there for a reason. If the loading takes long time for you, then maybe you have a malformed ("protected") archive.

Are you working on a map protector?