[PATCH] D94267: [PDB] Defer relocating .debug$S until commit time and parallelize it
Alexandre Ganea via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 11 13:03:18 PST 2021
aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.
LTGM, thanks!
I've checked several of our binaries, compared the PDB symbol stream, and except for extra S_SKIP records, it's all the same.
Some figures:
Large target (400 MB EXE, 2 GB PDB), MSVC 16.8 .OBJs, Unity files:
| | Link time | Peak commit |
| Before | 23 sec | 10.3 GB |
| After | 20.5 sec | 8.5 GB |
|
Summary
--------------------------------------------------------------------------------
4848 Input OBJ files (expanded from all cmd-line inputs)
61 PDB type server dependencies
40 Precomp OBJ dependencies
105084352 Input type records
5672204074 Input type records bytes
8275330 Merged TPI records
2939923 Merged IPI records
58997 Output PDB strings
8218778 Global symbol records
24183908 Module symbol records
2075680 Public symbol records
Same target as above, but compiled with Clang 11 .OBJs:
| | Link time | Peak commit |
| Before | 16.9 sec | 6.1 GB |
| After | 15.1 sec | 5 GB |
|
Summary
--------------------------------------------------------------------------------
4844 Input OBJ files (expanded from all cmd-line inputs)
61 PDB type server dependencies
0 Precomp OBJ dependencies
23827009 Input type records
1366100202 Input type records bytes
6548058 Merged TPI records
2588948 Merged IPI records
58643 Output PDB strings
4454924 Global symbol records
42985893 Module symbol records
1906582 Public symbol records
For smaller targets (like a game retail target), this patch consistently saves 1.3 sec.
================
Comment at: lld/COFF/PDB.cpp:125
+ void analyzeSymbolSubsection(SectionChunk *debugChunk,
+ uint32_t &moduleStreamSize,
+ uint32_t &nextRelocIndex,
----------------
s/moduleStreamSize/moduleSymOffset/ to match the definition.
================
Comment at: lld/COFF/PDB.cpp:218
+ /// Includes record realignment.
+ uint32_t moduleStreamSize = 4;
+
----------------
It's a bit strange that '4' means to do nothing in `finish()` but I understand that it makes the logic in `analyzeSymbolSubsection()` less complicated.
================
Comment at: lld/COFF/PDB.cpp:460
case SymbolKind::S_LPROC32:
+ case SymbolKind::S_GPROC32_ID:
+ case SymbolKind::S_LPROC32_ID:
----------------
Was this a divergence from MSVC link.exe, or was it handled somewhere else before your patch?
================
Comment at: lld/COFF/PDB.cpp:1510
+ // Write ptrEnd for the S_THUNK32.
+ ulittle32_t *pEnd = reinterpret_cast<ulittle32_t *>(
+ const_cast<uint8_t *>(&newSym.data()[8]));
----------------
I can't say I like poking plain memory without going through structured data, it makes the code less reader-friendly. It's a pity we don't have definitions for serialized structures :-( Nothing you can do now I guess.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94267/new/
https://reviews.llvm.org/D94267
More information about the llvm-commits
mailing list