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

[Eric Christopher]

FWIW this seems like a good idea to me (and you actually forgot the CC
afaict).

-eric

On Mon Feb 16 2015 at 11:30:58 AM Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150216/1b81d036/attachment.html>