<div dir="ltr">FWIW this seems like a good idea to me (and you actually forgot the CC afaict).<br><br><div>-eric</div></div><br><div class="gmail_quote">On Mon Feb 16 2015 at 11:30:58 AM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hans,<br>
<br>
If it's at all possible, this commit should be pulled into 3.6.x<br>
before you release 3.6.0.<br>
<br>
By now I've rewritten all the code for bitcode reading/writing of<br>
metadata, so you probably just need my approval.  I've also CC'ed<br>
Joe Abbey, who is listed in CODE_OWNERS.txt, in case you want his<br>
approval as well.<br>
<br>
Sorry for the late patch...<br>
<br>
> On 2015-Feb-16, at 11:18, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:<br>
><br>
> Author: dexonsmith<br>
> Date: Mon Feb 16 13:18:01 2015<br>
> New Revision: 229421<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=229421&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=229421&view=rev</a><br>
> Log:<br>
> Bitcode: Fix major regression: large files w/ debug info<br>
><br>
> The metadata/value split introduced a major regression reading large<br>
> bitcode files that contain debug info (or other cyclic (non-self<br>
> reference) metadata graphs).  For the first time in a while, I dropped<br>
> from libLTO.dylib down to `llvm-lto` with a non-trivial bitcode file<br>
> (~350MB), and I hit this when reading the result of ld64's `-save-temps`<br>
> in `llvm-lto`.<br>
><br>
> Here's pseudo-code for what was going on:<br>
><br>
>    read-main-metadata-block:<br>
>      for each md:<br>
>        if has-fwd-ref: // Only true for cyclic graphs.<br>
>          any-fwd-refs <- true<br>
>      if any-fwd-refs:<br>
>        foreach md:<br>
>          resolve-cycles(md) // Handle cycles.<br>
><br>
>    foreach function:<br>
>      read-function-metadata-block: // Such as !alias, !loop<br>
>        if any-fwd-refs:<br>
>          foreach md: // (all metadata, not just this block)<br>
>            resolve-cycles(md) // A no-op, but the loop is expensive!!<br>
><br>
> This commit resets the `AnyFwdRefs` flag to `false`.  This on its own<br>
> was enough to change my Release+Asserts `llvm-lto` time for reading this<br>
> bitcode from over 20 minutes (I gave up on it) to 20 seconds.  I've gone<br>
> further by tracking the min/max metadata forward-references in a<br>
> metadata block.  This protects against a schema that has lots of<br>
> functions that each reference their own metadata cycle.<br>
><br>
> Unfortunately, this regression is in the 3.6 branch as well.<br>
><br>
> Modified:<br>
>    llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.cpp<br>
>    llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.h<br>
><br>
> Modified: llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=229421&r1=229420&r2=229421&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Bitcode/Reader/BitcodeReader.<u></u>cpp?rev=229421&r1=229420&r2=<u></u>229421&view=diff</a><br>
> ==============================<u></u>==============================<u></u>==================<br>
> --- llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.cpp (original)<br>
> +++ llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.cpp Mon Feb 16 13:18:01 2015<br>
> @@ -555,9 +555,17 @@ Metadata *BitcodeReaderMDValueList::<u></u>getV<br>
>   if (Metadata *MD = MDValuePtrs[Idx])<br>
>     return MD;<br>
><br>
> -  // Create and return a placeholder, which will later be RAUW'd.<br>
> -  AnyFwdRefs = true;<br>
> +  // Track forward refs to be resolved later.<br>
> +  if (AnyFwdRefs) {<br>
> +    MinFwdRef = std::min(MinFwdRef, Idx);<br>
> +    MaxFwdRef = std::max(MaxFwdRef, Idx);<br>
> +  } else {<br>
> +    AnyFwdRefs = true;<br>
> +    MinFwdRef = MaxFwdRef = Idx;<br>
> +  }<br>
>   ++NumFwdRefs;<br>
> +<br>
> +  // Create and return a placeholder, which will later be RAUW'd.<br>
>   Metadata *MD = MDNode::getTemporary(Context, None).release();<br>
>   MDValuePtrs[Idx].reset(MD);<br>
>   return MD;<br>
> @@ -573,7 +581,8 @@ void BitcodeReaderMDValueList::<u></u>tryToReso<br>
>     return;<br>
><br>
>   // Resolve any cycles.<br>
> -  for (auto &MD : MDValuePtrs) {<br>
> +  for (unsigned I = MinFwdRef, E = MaxFwdRef + 1; I != E; ++I) {<br>
> +    auto &MD = MDValuePtrs[I];<br>
>     auto *N = dyn_cast_or_null<MDNode>(MD);<br>
>     if (!N)<br>
>       continue;<br>
> @@ -581,6 +590,9 @@ void BitcodeReaderMDValueList::<u></u>tryToReso<br>
>     assert(!N->isTemporary() && "Unexpected forward reference");<br>
>     N->resolveCycles();<br>
>   }<br>
> +<br>
> +  // Make sure we return early again until there's another forward ref.<br>
> +  AnyFwdRefs = false;<br>
> }<br>
><br>
> Type *BitcodeReader::getTypeByID(<u></u>unsigned ID) {<br>
><br>
> Modified: llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.h?rev=229421&r1=229420&r2=229421&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Bitcode/Reader/BitcodeReader.<u></u>h?rev=229421&r1=229420&r2=<u></u>229421&view=diff</a><br>
> ==============================<u></u>==============================<u></u>==================<br>
> --- llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.h (original)<br>
> +++ llvm/trunk/lib/Bitcode/Reader/<u></u>BitcodeReader.h Mon Feb 16 13:18:01 2015<br>
> @@ -99,6 +99,8 @@ public:<br>
> class BitcodeReaderMDValueList {<br>
>   unsigned NumFwdRefs;<br>
>   bool AnyFwdRefs;<br>
> +  unsigned MinFwdRef;<br>
> +  unsigned MaxFwdRef;<br>
>   std::vector<TrackingMDRef> MDValuePtrs;<br>
><br>
>   LLVMContext &Context;<br>
><br>
><br>
> ______________________________<u></u>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div>