[PATCH] D79467: [PDB] Optimize public symbol processing

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked an inline comment as done.
rnk added a comment.

In D79467#2022848 <https://reviews.llvm.org/D79467#2022848>, @aganea wrote:

> Can you diff the full output of `microsoft-pdb\cvdump\cvdump.exe your_exe.pdb` and `llvm-pdbutil dump -all your_exe.pdb` before and after this patch? (with any large exe). This can be a bit tedious because the text dump is very large, but you can at least validate that things are still right (you're probably already doing this).


There is a diff in the globals stream due to different global hash bucket sorting. An example hunk looks like this:

  $ diff -u lld*-pubs.txt | head -40
  --- lld1-pubs.txt       2020-05-06 17:25:41.132778100 -0700
  +++ lld2-pubs.txt       2020-05-06 17:25:37.352925100 -0700
  @@ -197,11 +197,11 @@
              original type = 0x111F9D
     22547392 | S_UDT [size = 12] `HDC`
              original type = 0xC5A17
  -  23582124 | S_GDATA32 [size = 28] `__newclmap`
  +  23513760 | S_GDATA32 [size = 28] `__newclmap`
              type = 0x11445B (), addr = 0002:3648064
     23517196 | S_GDATA32 [size = 28] `__newclmap`
              type = 0x114534 (), addr = 0002:3648064
  -  23513760 | S_GDATA32 [size = 28] `__newclmap`
  +  23582124 | S_GDATA32 [size = 28] `__newclmap`
              type = 0x11445B (), addr = 0002:3648064
     11925196 | S_CONSTANT [size = 24] `SHRD32rri8`
              type = 0x24E8D (llvm::X86::<unnamed-tag>), value = 2704
  @@ -1595,10 +1595,10 @@

The original code uses llvm::sort with gsiRecordLess, which is unstable, although deterministic. I modified the code to sort by symoffset to stabilize the output, but the new stable order doesn't match the old unstable order.

Other than that, all the streams and PDBs have the same size.

I checked the padding, and I believe the old publics are padded with zero bytes:
https://github.com/llvm/llvm-project/blob/01fc85dc9618394868b795c5087d9da03df9c58b/llvm/lib/DebugInfo/CodeView/SymbolRecordMapping.cpp#L42



================
Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:207
+  // algorithm used here corredsponds to the function
+  // caseInsensitiveComparePchPchCchCch in the reference implementation.
+  auto BucketCmp = [](const BulkPublic &L, const BulkPublic &R) {
----------------
aganea wrote:
> Maybe provide a link to the reference implemention repo? (ie.https://github.com/microsoft/microsoft-pdb)
I put one in the file header comment like the other files in here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79467