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

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 2 inline comments as done.
rnk added a comment.

In D79467#2022660 <https://reviews.llvm.org/D79467#2022660>, @hans wrote:

> This looks good as far as I can tell. Is there good test coverage here, e.g. if there's an off-by-one somewhere, would it be caught?


I will say that the existing tests caught many issues, and that gives me some confidence in this. All the hash bucket table stuff is pretty well tested.



================
Comment at: lld/COFF/PDB.cpp:1708
   pdb.addNamedStreams();
+  pdb.addPublicsToPDB();
 
----------------
hans wrote:
> Before it ran at the end of addObjectsToPDB(). Does the order matter?
My aim was to improve cache locality, but I'm not sure it matters. Previously we would add and sort the publics, then add the section contributions (slow, flushes cache), then commit the publics to disk. I figured this order would be better. In isolation, I don't think I was able to measure a difference above the noise though. In any case, the publics aren't really part of adding objects.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:117
+  // Zero the null terminator and remaining bytes.
+  memset(&NameMem[NameLen], 0, Size - sizeof(PublicSym32Layout) - NameLen);
+  return CVSymbol(makeArrayRef(reinterpret_cast<uint8_t *>(Mem), Size));
----------------
Speaking of undertested things, these might need to be LF_PAD bytes. I will double check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79467