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

enable debug information in MSVC release build #3873

Merged
merged 2 commits into from Jun 8, 2021

Conversation

krishty
Copy link
Contributor

@krishty krishty commented May 4, 2021

No effect on runtime speed/size. Slightly slower link time, but debugging experience improves by a million fold.

  • /Zi – Store debug information in a .pdb file, not directly in the binary
  • /DEBUG:FULL – generate debug information during link
  • /PDBALTPATH:%_PDB% – do not store the file system path of the .pdb, just the filename and hash (no disclose paths on distribution)
  • /OPT:REF /OPT:ICF – remove unreferenced functions and fold identical functions (this was enabled before, but is implictly disabled by /DEBUG:FULL)

No effect on runtime speed/size. Slightly slower link time, but debugging experience improves by a million times.

- /Zi – Store debug information in a .pdb file, not directly in the DLL/EXE
- /DEBUG:FULL – generate debug information during link
- /PDBALTPATH:%_PDB% – do not store the file system path of the .pdb, just the filename and hash (no disclose paths on distribution)
- /OPT:REF /OPT:ICF – remove unreferenced functions and fold identical functions (this was enabled before, but requires explicit enabling if /DEBUG:FULL is specified)
@RichardTea
Copy link
Contributor

RichardTea commented May 9, 2021

This is not required, and should be withdrawn.

Cmake recommends using the default created "ReleaseWithDebug" project configuration for MSVC Release builds that have debug information.

Messing with the default CXXFLAGS is almost always a really bad idea. It nearly always means you've misunderstood something.

@krishty
Copy link
Contributor Author

krishty commented May 9, 2021

Cmake recommends using the default created "ReleaseWithDebug" project configuration for MSVC Release builds that have debug information.

Then I’ll open an issue for Assimp’s RelWithDebInfo being broken, because it embeds the debug information into the executable and has some optimizations disabled, therefore it’s very impractical for distribution.

Compare
RelWithDebInfo: 11 MiB DLL with 3 MiB PDB, optimized for Edit & Continue (incrementally linked)
Release: 5 MiB DLL, all optimizations, but no way to debug it
Release with this patch: 5 MiB DLL, all optimizations, 50 MiB PDB for offline debugging

@RichardTea
Copy link
Contributor

RichardTea commented May 9, 2021

I think that's a fairly recent regression then, I've been using RelWithDebug for a few years.
I am more than three/four months behind, I think.

@RichardTea
Copy link
Contributor

RichardTea commented May 11, 2021

Well, I'm wrong.

It's a cmake issue. That's tedious.
See here: https://gitlab.kitware.com/cmake/cmake/-/issues/20812

BTW, /DEBUG:FULL is implied by command-line builds, though best to set it anyway.

With MSVC2019 (Draco enabled) I get these sizes for assimp-vc142-mt(d).dll:

  • Release: 5.2MB
  • RelWithDebugInfo: 7.4MB and a 52MB PDB.
  • Debug: 18.3MB and a 79MB PDB

I withdraw my objection and will go poke Kitware

@RichardTea
Copy link
Contributor

Note:
Definitely also need: /INCREMENTAL:NO

@krishty
Copy link
Contributor Author

krishty commented May 11, 2021

Note:
Definitely also need: /INCREMENTAL:NO

No, this is implied by /OPT:REF /OPT:ICF. But specifying it explicitly could make matters more clear.

@kimkulling kimkulling merged commit c58fe0c into assimp:master Jun 8, 2021
@kimkulling
Copy link
Member

Merged, thanks a lot for your contribution.

@krishty krishty deleted the improve-msvc-switches branch June 22, 2021 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants