Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScanForFunctions: Speed up game loading by only trying to insert the newly found functions into the symbol map. #12652

Merged
merged 1 commit into from
Feb 29, 2020

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Feb 29, 2020

I wondered why ScanForFunctions was so incredibly slow in debug, and the answer wasn't just all the std::map manipulation in g_symbolMap->AddFunction...

It's not very clear in the code (needs some renaming and cleanup) but functions is a global vector here and at the end of ScanForFunctions, we'd loop through the entire thing and add all of them to the symbol map. ScanForFunctions is often called many times during load due to lots of little sections in the binary (most of which actually don't contain functions, it turns out) so we ended up trying to add the same large vector of symbols many times.

Now, we just try to add the newly found functions instead of all of the old ones again.

It's no longer somewhat painful to start big games in debug mode, and it is noticeably faster to start up games in general. I don't see how this could break anything, but @unknownbrackets might want a quick look?

(you'd think that the "foundInSymbolMap" thing would prevent this, but it didn't anticipate trying to add all the old symbols repeatedly, which won't have that flag...)

@hrydgard hrydgard added this to the v1.10.0 milestone Feb 29, 2020
@hrydgard
Copy link
Owner Author

Gonna go ahead and merge - I can't think of any way this would break anything.

@hrydgard hrydgard merged commit 3a00e13 into master Feb 29, 2020
@hrydgard hrydgard deleted the game-load-speedup branch February 29, 2020 09:32
@unknownbrackets
Copy link
Collaborator

Yeah, this makes sense.

-[Unknown]

@ghost ghost mentioned this pull request Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants