[PATCH] D35738: Enable llvm-pdbutil to list enumerations using native PDB reader

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 24 10:21:40 PDT 2017


Imo, the most general solution would be to store every kind of cache we
might ever create in the Session object, and various symbolic such as this
go through the session to get the right info. This way if someone calls
getGlobalScope twice, they don't end up redoing this O(n) work.

Individual symbol objects can then either be a dumb facade over the
session, or a dumb facade over the codeview record, whichever makes sense.

In this case i don't think it makes sense to cache anything, there's an
inherent memory hit associated with doing so but the up front work required
to get the list of enum children is trivial, so i don't think a cache buys
you anything
On Mon, Jul 24, 2017 at 10:02 AM Reid Kleckner via Phabricator <
reviews at reviews.llvm.org> wrote:

> rnk added inline comments.
>
>
> ================
> Comment at: llvm/include/llvm/DebugInfo/PDB/Native/NativeEnumTypes.h:22
> +
> +class NativeEnumTypes : public IPDBEnumChildren<PDBSymbol> {
> +public:
> ----------------
> I'm wondering if we can get away with implementing these findChildren
> methods by building a vector of PDBSymbols up front, and then this class
> becomes a simple adapter from vector to IPDBEnumChildren. It seems to me if
> the caller calls findChildren, they're going to walk them all, so we can do
> O(n) work without slowing things down asymptotically.
>
>
> ================
> Comment at: llvm/lib/DebugInfo/PDB/Native/NativeExeSymbol.cpp:48-49
> +      const auto &Types = Tpi->types(&HadError);
> +      if (HadError)
> +        return nullptr;
> +      return std::unique_ptr<IPDBEnumSymbols>(
> ----------------
> This check will never fail. The optional HadError parameter is essentially
> an awkward way to thread error handling out of a range-based-for. The idea
> is that if the iterator encounters an error, it halts iteration and sets
> *HadError to true. The caller can check for failure outside the loop. We
> don't want to eagerly check for errors because that would require iterating
> over all the data.
>
> Does this method get called often or early? I'm wondering if it would be
> better to iterate and filter the type stream up front and make a vector.
>
>
> https://reviews.llvm.org/D35738
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170724/b6bacc4f/attachment.html>


More information about the llvm-commits mailing list