[PATCH] D52283: [PDB] Add the ability to resolve forward references

[Zachary Turner via llvm-commits]

It’s true that nested containers don’t usually perform well, but in this
case the outer vector *never* resizes. So it seems like it should be ok?
On Wed, Sep 19, 2018 at 4:08 PM Reid Kleckner via Phabricator <
reviews at reviews.llvm.org> wrote:

> rnk added inline comments.
>
>
> ================
> Comment at: llvm/include/llvm/DebugInfo/PDB/Native/TpiHashing.h:60
> +    codeview::UnionRecord Union;
> +  };
> +
> ----------------
> You will probably have to name the union field for this to build with all
> supported compilers.
>
>
> ================
> Comment at: llvm/include/llvm/DebugInfo/PDB/Native/TpiStream.h:87
>
> +  std::vector<std::vector<codeview::TypeIndex>> HashMap;
> +
> ----------------
> Nested C++ containers don't usually perform the best. Here come some
> obnoxious data structure micro-optimization suggestions down below!
>
>
>
> ================
> Comment at: llvm/lib/DebugInfo/PDB/Native/TpiStream.cpp:146
>
> +void TpiStream::buildHashMap() {
> +  if (!HashMap.empty())
> ----------------
> How about this:
> 1. Allocate an array of TypeIndex of size `TypeIndexEnd - TypeIndexBegin`,
> i.e. an array of all the type indices in the hash table.
> 2. Make HashMap a `std::vector<std::pair<unsigned, unsigned>>`, where
> these are begin/end indices of ranges into the previous array. We can write
> a helper that takes a hash value and returns an `ArrayRef<TypeIndex>` using
> these two arrays.
> 3. Build a temporary array of pairs of HashValue+TypeIndex, and sort it by
> hash value.
> 4. Iterate over the sorted array of hash values and type indices. Push
> each type index onto the array. When the hash value changes, store a pair
> of begin/end indices for the type indices that share this hash code into
> the HashMap.
>
>
> ================
> Comment at: llvm/lib/DebugInfo/PDB/Native/TpiStream.cpp:157
> +  while (TIB < TIE) {
> +    uint32_t HV = HashValues[TIB.toArrayIndex()];
> +    HashMap[HV].push_back(TIB++);
> ----------------
> If `HV >= Header->NumHashBuckets`, I would report an error or just drop
> the type. We could hash the type ourselves as a recovery, but it's a
> corrupt input file, anything but UB or crashing works.
>
>
> https://reviews.llvm.org/D52283
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180919/fa76ddae/attachment.html>