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 wjp, 10 months ago

I see this same slowdown. Doing some quick experiments shows that almost all of the slowdown is explained by Path::equalsIgnoreCase() and Path::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?

comment:2 by mikrosk, 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.

Last edited 10 months ago by mikrosk (previous) (diff)
Note: See TracTickets for help on using tickets.