[PATCH] D87805: [PDB] Merge types in parallel when using ghashing

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a comment.

Some figures with this patch -- link times only:

|                             | VS2019 16.6.3 link.exe | LLD 10 (/DEBUG) | LLD 12 + this patch (/DEBUG) | LLD 12 + this patch (/DEBUG:GHASH) |
| Game - Editor MSVC          | 1 min 2 sec            | 51 sec          |                              | 27 sec                             |
| Game - Editor Clang         |                        | 28 sec          | 23 sec                       | 16 sec                             |
| Game - Engine Release Clang |                        | 20 sec          | 17 sec                       | 12 sec                             |
| Game - Engine Retail Clang  |                        | 17 sec          | 15 sec                       | 11 sec                             |
|



================
Comment at: lld/COFF/DebugTypes.cpp:745
+  // server.
+  TypeServerSource *tsSrc = check(getTypeServerSource());
+  tpiMap = tsSrc->tpiMap;
----------------
This needs to be:
```
  Expected<TypeServerSource *> tsSrc = getTypeServerSource();
  if (!tsSrc)
    return; // ignore errors at this point.
```
Since a missing PDB is not en error, we just won't have types & symbols for that .OBJ - and we're already handling that later in `mergeDebugT`. Could you also please modify `pdb-type-server-missing.yaml` as it lacks `/DEBUG:GHASH` coverage, which should catch this case?


================
Comment at: lld/COFF/DebugTypes.cpp:903
+
+  // FIXME: The low bytes of SHA1 have low entropy for short records, which
+  // type records are. Swap the byte order for better entropy. A better ghash
----------------
Note for future developement: It would be nice to support other kinds of hashers in `.debug$H`. SHA1 is not the best choice, see https://reviews.llvm.org/D55585#1356894 - xxHash64 seems like a better solution. I've also tried MeowHash, and since it uses AES instructions it run pretty much at memory bandwidth speed: https://github.com/cmuratori/meow_hash


================
Comment at: lld/COFF/DebugTypes.cpp:953
+  // Load ghashes. Do type servers and PCH objects first.
+  parallelForEach(TpiSource::instances, [&](TpiSource *source) {
+    if (source->isDependency())
----------------
Can you please add a timer for this part? (just the ghash generation for all files)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87805/new/

https://reviews.llvm.org/D87805