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

FixedSizeQueue: Work around GCC generating large amounts of debug info #8386

Merged
merged 1 commit into from Oct 9, 2019

Conversation

@stenzek
Copy link
Contributor

commented Oct 6, 2019

The storage = {}; statement in the clear() function causes GCC to blow up in memory usage when compiling LogWidget.cpp (approx. 16GB on my machine), and generate a 3.1GB assembly file in the RelWithDebInfo config. The dolphin-emu binary grows from ~300MB to ~900MB.

Not certain if this is a gcc bug or working as intended... I created a small repro program (albeit with a larger array size) which exhibits the same issue, and crashes gcc: https://pastebin.com/raw/iXrQEv0j

@Antidote

This comment has been minimized.

Copy link

commented Oct 6, 2019

Definitely a GCC bug, I'd report it upstream.

@stenzek

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2019

This is a regression from #8337, although as I said I'm not sure if the code is at fault here. Any thoughts, @CookiePLMonster?

@CookiePLMonster

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

I don't know enough about compilers but this indeed doesn't make any sense.

I know this is a lot to ask but can you pack and upload a compiled binary and debug info? I'd like to disassemble it.

Edit: hold on with that, I'll first play around with compiler explorer. What is your exact GCC version?

@stenzek

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2019

Edit: hold on with that, I'll first play around with compiler explorer. What is your exact GCC version?

gcc version 9.2.1 20190909 (Ubuntu 9.2.1-8ubuntu1)

@Antidote

This comment has been minimized.

Copy link

commented Oct 6, 2019

I'm getting the same issue with g++ (GCC) 9.1.0 on Arch
It compiles fine with clang, but segfaults

@stenzek

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2019

Looks like GCC is compiling the empty-initializer-list assignment into an unrolled loop initializing all elements, which generates a decent amount of code, but that's where the extra size in the debug info is coming from. clang appears to compile it as an actual loop...

@CookiePLMonster

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

If that is the case then I think it would be best to use storage.fill on this. Should hopefully not generate code as stupid.

@CookiePLMonster

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

// The clear of non-trivial objects previously used "storage = {}". However, this causes GCC
// to take a very long time to compile the file/function, as well as generating huge amounts
// of debug information (~2GB object file, ~600MB of debug info).
while (count > 0)

This comment has been minimized.

Copy link
@CookiePLMonster

CookiePLMonster Oct 6, 2019

Contributor
Suggested change
while (count > 0)
storage.fill({});

I played around with Compiler Explorer and this is semantically equivalent to initializing with an empty initializer list and produces sane code.

This comment has been minimized.

Copy link
@jordan-woyak

jordan-woyak Oct 6, 2019

Member

Using .fill would be nicer if it does work around the bug.

This comment has been minimized.

Copy link
@stenzek

stenzek Oct 6, 2019

Author Contributor

As I mentioned on IRC, filling would have the effect of always default constructing N (5000 in the log case) objects vs count objects. For the clear button this isn't a big deal but it might matter with other users of the class.

This comment has been minimized.

Copy link
@CookiePLMonster

CookiePLMonster Oct 8, 2019

Contributor

In this case I'm cool with it.

@stenzek stenzek merged commit 6e613f4 into dolphin-emu:master Oct 9, 2019
8 checks passed
8 checks passed
default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
@stenzek stenzek deleted the stenzek:gcc-array-workaround branch Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.