[PATCH] D85379: Improve dropTriviallyDeadConstantArrays time cost ratio from 17% to 4%

[Stephan Z via Phabricator via llvm-commits]

stephan.yichao.zhao created this revision.
stephan.yichao.zhao added reviewers: mehdi_amini, pcc, tejohnson.
Herald added subscribers: llvm-commits, dexonsmith, hiraditya.
Herald added a project: LLVM.
stephan.yichao.zhao requested review of this revision.

The history of dropTriviallyDeadConstantArrays is like this. Because the appending linkage uses too much memory (http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150105/251381.html), dropTriviallyDeadConstantArrays was introduced (https://reviews.llvm.org/rG81f385b0c6ea37dd7195a65be162c75bbdef29d2) to release unused constant arrays. Recently, dropTriviallyDeadConstantArrays was improved (https://reviews.llvm.org/rG81f385b0c6ea37dd7195a65be162c75bbdef29d2) to reduce its quadratic cost.

Our recent LTO profiling shows that when a target is large, 15-20% of time cost is from the SetVector::insert called by dropTriviallyDeadConstantArrays.

A large application has hundreds or thousands of modules; each module calls dropTriviallyDeadConstantArrays once for cleaning up tens of thousands of ConstantArrays a module has. In those ConstantArrays, usually around 5 can be deleted; a very very few deleted ConstantArrays reference other ConstantArrays: less than 10 out of millions.

Given this, the cost of SetVector::insert is mainly from the construction of WorkList from ArrayConstants. This motivated the fix that iterates ArrayConstants directly, and uses WorkList only when necessary.

Our evaluation shows that

1. The time cost ratio of dropTriviallyDeadConstantArrays is reduced from 15-17% to 4-6%.
2. For targets with LTO time > 20min, the time reduction is about 20%.
3. No observable performance impact for build without using LTO.

See the description of cl/324220676 for evaluation in detail.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85379

Files:
  llvm/lib/IR/LLVMContextImpl.cpp


Index: llvm/lib/IR/LLVMContextImpl.cpp
===================================================================
--- llvm/lib/IR/LLVMContextImpl.cpp
+++ llvm/lib/IR/LLVMContextImpl.cpp
@@ -129,18 +129,46 @@
 }
 
 void LLVMContextImpl::dropTriviallyDeadConstantArrays() {
-  SmallSetVector<ConstantArray *, 4> WorkList(ArrayConstants.begin(),
-                                              ArrayConstants.end());
+  // If a ConstantArray is destroyed, its ConstantArray operands could be dead
+  // too. Use a work list to keep track of such operands.
+  SmallSetVector<ConstantArray *, 4> WorkList;
+  // ArrayConstants can contain C1 and C2 in sequence where C2 is one of C1's
+  // operands. After C1 is destroyed, C2 is added into the work list. Before
+  // processing the C2 in the worklist, C2 is destroyed. Use a set to track of
+  // ConstantArrays deleted when iterating ArrayConstants.
+  //
+  // If ArrayConstants are copied into the work list first, the above issue does
+  // not occur because SmallSetVector keeps elements unique. However, for a
+  // large application, such a copy can take 20% LTO time. In pratice, a very
+  // few operands of a destroyed ConstantArray is dead. Therefore, we first
+  // iterate ArrayConstants, and use WorkList for only operands of type
+  // ConstantArray.
+  //
+  // In practics, a very few ConstantArrays are destroyed from ArrayConstants in
+  // the initial pass. So the sizes of WorkList and Deleted are much smaller
+  // than the size of ArrayConstants.
+  SmallPtrSet<ConstantArray *, 4> Deleted;
+
+  for (auto I = ArrayConstants.begin(), E = ArrayConstants.end(); I != E;) {
+    auto *C = *I++;
+    if (!C->use_empty())
+      continue;
+    for (const Use &Op : C->operands())
+      if (auto *COp = dyn_cast<ConstantArray>(Op))
+        WorkList.insert(COp);
+    // This does not invalidate ArrayConstants's iterator.
+    C->destroyConstant();
+    Deleted.insert(C);
+  }
 
   while (!WorkList.empty()) {
     ConstantArray *C = WorkList.pop_back_val();
-    if (C->use_empty()) {
-      for (const Use &Op : C->operands()) {
-        if (auto *COp = dyn_cast<ConstantArray>(Op))
-          WorkList.insert(COp);
-      }
-      C->destroyConstant();
-    }
+    if (!C->use_empty() || Deleted.contains(C))
+      continue;
+    for (const Use &Op : C->operands())
+      if (auto *COp = dyn_cast<ConstantArray>(Op))
+        WorkList.insert(COp);
+    C->destroyConstant();
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85379.283445.patch
Type: text/x-patch
Size: 2451 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200806/225ef995/attachment.bin>