Opened 10 months ago
Last modified 10 months ago
#14853 new defect
COMMON: Rework path handling causes performance regression
Reported by: | mikrosk | Owned by: | |
---|---|---|---|
Priority: | normal | Component: | Common |
Version: | Keywords: | performance, regression | |
Cc: | Game: |
Description
https://github.com/scummvm/scummvm/pull/5467 introduced a new (safer) way how to handle file paths.
I was looking forward to this change because I tested its early iteration and it brought quite visible speedup when adding new games.
Surprisingly, when the dust settled after the 2.8.0 release we have found out that our Atari backend regressed in master: adding a game with a lot of files (e.g. Dreamweb with ~100 files) could lead to nearly twice as slow game detection.
Some numbers:
Atari backend, attempting to add a folder with 1000 generated files:
- 2.8.0: 33s
- master: 58s
Linux SDL backend, attempting to add a folder with 100 000 generated files ("for i in $(seq 99999); do touch $(printf "%04d" $i); done"):
- 2.8.0: 17s
- master: 28s
Naturally, no game is going to have 100 000 files, this is just to demonstrate that the powerful CPU can hide even a performance drop to 60% of the original speed.
Fortunately, thanks to LP's responsible approach, each commit is compilable and testable: so in this case the offending commit is https://github.com/scummvm/scummvm/commit/645a35c0 (not so surprising as it directly manipulates the detection code).
Change History (2)
comment:1 by , 10 months ago
comment:2 by , 10 months ago
With https://github.com/scummvm/scummvm/pull/5621 merged, I'm happy to report that adding/detecting Dreamweb is now much faster than in 2.8! So the original speed boost I remembered is definitely present/visible.
However, the when it comes to hashing/comparing, there's still some performance decrease if the amount of files is too big: adding a folder with 999 (generated) files is still about 20% slower than in 2.8. The aforementioned PR definitely helped but it's still slower. I can provide a detailed call graph if it helps.
I see this same slowdown. Doing some quick experiments shows that almost all of the slowdown is explained by
Path::equalsIgnoreCase()
andPath::hashIgnoreCase()
.They seem to be going through the full string three times instead of just once now (strchr for the separator, String constructor, and then the actual hashing or comparison), also creating a bunch of temporary strings in the process.
May be worth it to add a fast specialization for these for the non-escaped variants?