Inline some of the most called functions in SCI and SCUMM #4852
Conversation
|
That's great! Nice job! The SCUMM parts looks good to go! |
|
Overall this looks good to me. For fun, I compared msvc release builds, the SCI changes add 4k. I appreciate the profiling details, my first response to the PR title was "...but we're speed throttled?", makes much more sense given the machines you're targeting. =) What's the benefit of moving declarations into a new Don't worry about the FIXME on making the SCI version global not a global; that would be a huge undertaking and it's not really a problem. Might as well get some speed out of it. =) A while ago I reversed a ScummVM msvc exe to see what a fork that we didn't have source for had done. Everywhere I looked I saw Also, commit messages need SCI and SCUMM: prefixes |
mikrosk
force-pushed
the
scumm_sci_inlines
branch
from
53fa3cc to
2091c48
Compare
|
Pushed new changes.
Initially I thought it is a good idea to have all version-related functions together. However you are right, the versions are more related to resource handling than some generic version checking so I've moved the two functions into As for I've also fixed the cosmetic requests. |
|
Thank you, makes sense, and I like the minimalism of the changes. |
4 participants
When profiling a few games, we have spotted some functions which repeatedly made it to the top ten most called / cpu eating functions (on a 32 MHz hardware, so that explains why those numbers are so high...)
From the SCI camp we tested Gabriel Knight, Larry 2, Larry 7 and Sierra's Christmas Card 1988. The most CPU intensive functions (shown on Larry 2 intro but it is similar for the others):
8.30% of all spent CPU cycles:
Sci::reg_t::getOffset() const6.08% of all spent CPU cycles:
Sci::getSciVersion()1.80% of all spent CPU cycles:
Sci::reg_t::setOffset(unsigned int)1.25% of all spent CPU cycles:
Sci::Script::offsetIsObject(unsigned int) constAfter applying this PR, the total
run_vm()cost went from 0.00940s / call, down to 0.00672s / call, which is 28.5% improvement!During a run of Full Throttle,
testGfxUsageBit()calls account for 6.2% of all function calls (!) and ~2% of used cycles.The
getSciVersioncommit is perhaps the most interesting for review purposes; I was thinking about doing what theFIXMEsuggests but that wouldn't be such a win for performance as it could be inlined, sure, but also it would be replaced with another indirect jump caused byg_sci->getSciVersion()so I've taken the most efficient path even if it doesn't improve the code.