[llvm] r229421 - Bitcode: Fix major regression: large files w/ debug info

[Duncan P. N. Exon Smith]

Hans,

If it's at all possible, this commit should be pulled into 3.6.x
before you release 3.6.0.

By now I've rewritten all the code for bitcode reading/writing of
metadata, so you probably just need my approval.  I've also CC'ed
Joe Abbey, who is listed in CODE_OWNERS.txt, in case you want his
approval as well.

Sorry for the late patch...

> On 2015-Feb-16, at 11:18, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> Author: dexonsmith
> Date: Mon Feb 16 13:18:01 2015
> New Revision: 229421
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=229421&view=rev
> Log:
> Bitcode: Fix major regression: large files w/ debug info
> 
> The metadata/value split introduced a major regression reading large
> bitcode files that contain debug info (or other cyclic (non-self
> reference) metadata graphs).  For the first time in a while, I dropped
> from libLTO.dylib down to `llvm-lto` with a non-trivial bitcode file
> (~350MB), and I hit this when reading the result of ld64's `-save-temps`
> in `llvm-lto`.
> 
> Here's pseudo-code for what was going on:
> 
>    read-main-metadata-block:
>      for each md:
>        if has-fwd-ref: // Only true for cyclic graphs.
>          any-fwd-refs <- true
>      if any-fwd-refs:
>        foreach md:
>          resolve-cycles(md) // Handle cycles.
> 
>    foreach function:
>      read-function-metadata-block: // Such as !alias, !loop
>        if any-fwd-refs:
>          foreach md: // (all metadata, not just this block)
>            resolve-cycles(md) // A no-op, but the loop is expensive!!
> 
> This commit resets the `AnyFwdRefs` flag to `false`.  This on its own
> was enough to change my Release+Asserts `llvm-lto` time for reading this
> bitcode from over 20 minutes (I gave up on it) to 20 seconds.  I've gone
> further by tracking the min/max metadata forward-references in a
> metadata block.  This protects against a schema that has lots of
> functions that each reference their own metadata cycle.
> 
> Unfortunately, this regression is in the 3.6 branch as well.
> 
> Modified:
>    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.h
> 
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=229421&r1=229420&r2=229421&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Mon Feb 16 13:18:01 2015
> @@ -555,9 +555,17 @@ Metadata *BitcodeReaderMDValueList::getV
>   if (Metadata *MD = MDValuePtrs[Idx])
>     return MD;
> 
> -  // Create and return a placeholder, which will later be RAUW'd.
> -  AnyFwdRefs = true;
> +  // Track forward refs to be resolved later.
> +  if (AnyFwdRefs) {
> +    MinFwdRef = std::min(MinFwdRef, Idx);
> +    MaxFwdRef = std::max(MaxFwdRef, Idx);
> +  } else {
> +    AnyFwdRefs = true;
> +    MinFwdRef = MaxFwdRef = Idx;
> +  }
>   ++NumFwdRefs;
> +
> +  // Create and return a placeholder, which will later be RAUW'd.
>   Metadata *MD = MDNode::getTemporary(Context, None).release();
>   MDValuePtrs[Idx].reset(MD);
>   return MD;
> @@ -573,7 +581,8 @@ void BitcodeReaderMDValueList::tryToReso
>     return;
> 
>   // Resolve any cycles.
> -  for (auto &MD : MDValuePtrs) {
> +  for (unsigned I = MinFwdRef, E = MaxFwdRef + 1; I != E; ++I) {
> +    auto &MD = MDValuePtrs[I];
>     auto *N = dyn_cast_or_null<MDNode>(MD);
>     if (!N)
>       continue;
> @@ -581,6 +590,9 @@ void BitcodeReaderMDValueList::tryToReso
>     assert(!N->isTemporary() && "Unexpected forward reference");
>     N->resolveCycles();
>   }
> +
> +  // Make sure we return early again until there's another forward ref.
> +  AnyFwdRefs = false;
> }
> 
> Type *BitcodeReader::getTypeByID(unsigned ID) {
> 
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.h?rev=229421&r1=229420&r2=229421&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.h (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.h Mon Feb 16 13:18:01 2015
> @@ -99,6 +99,8 @@ public:
> class BitcodeReaderMDValueList {
>   unsigned NumFwdRefs;
>   bool AnyFwdRefs;
> +  unsigned MinFwdRef;
> +  unsigned MaxFwdRef;
>   std::vector<TrackingMDRef> MDValuePtrs;
> 
>   LLVMContext &Context;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits