[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

As noted by @nikic, D126061 <https://reviews.llvm.org/D126061> causes a compile time regression of about
0.5% on -O0 builds:

http://llvm-compile-time-tracker.com/compare.php?from=7acc88be0312c721bc082ed9934e381d297f4707&to=8c7b64b5ae2a09027c38db969a04fc9ddd0cd6bb&stat=instructions

This happens because, in a number of places, D126061 <https://reviews.llvm.org/D126061> creates an
additional local variable of type `ParsedAttributes`. In the large
majority of cases, no attributes are added to this `ParsedAttributes`,
but it turns out that creating an empty `ParsedAttributes`, then
destroying it is a relatively expensive operation.

The reason for this is because `AttributePool` uses a `TinyPtrVector` as
its underlying vector class, and the technique that `TinyPtrVector`
employs to achieve its extreme memory frugality makes the `begin()` and
`end()` member functions relatively slow. The `ParsedAttributes`
destructor iterates over the attributes in its `AttributePool`, and this
is a relatively expensive operation because `TinyPtrVector`'s `begin()` and
`end()` are relatively slow.

The fix for this is to replace `TinyPtrVector` in `ParsedAttributes` and
`AttributePool` with `SmallVector`. `ParsedAttributes` and
`AttributePool` objects are only ever allocated on the stack (they're
not part of the AST), and only a small number of these objects are live
at any given time, so they don't need the extreme memory frugality of
`TinyPtrVector`.

I've confirmed with valgrind that this patch does not increase heap
memory usage, and it actually makes compiles slightly faster than they
were before D126061 <https://reviews.llvm.org/D126061>.

Here are instruction count measurements (obtained with callgrind)
running `clang -c MultiSource/Applications/JM/lencod/parsetcommon.c`
(a file from llvm-test-suite that exhibited a particularly large
compile-time regression):

7acc88be0312c721bc082ed9934e381d297f4707 <https://reviews.llvm.org/rG7acc88be0312c721bc082ed9934e381d297f4707>
(baseline one commit before D126061 <https://reviews.llvm.org/D126061> landed)
102,280,068 instructions

8c7b64b5ae2a09027c38db969a04fc9ddd0cd6bb <https://reviews.llvm.org/rG8c7b64b5ae2a09027c38db969a04fc9ddd0cd6bb>
(the patch that landed D126061 <https://reviews.llvm.org/D126061>)
103,289,454 instructions
(+0.99% relative to baseline)

This patch applied onto
8c7b64b5ae2a09027c38db969a04fc9ddd0cd6bb <https://reviews.llvm.org/rG8c7b64b5ae2a09027c38db969a04fc9ddd0cd6bb>
101,117,584 instructions
(-1.14% relative to baseline)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128097

Files:
  clang/include/clang/Sema/ParsedAttr.h


Index: clang/include/clang/Sema/ParsedAttr.h
===================================================================
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -21,7 +21,6 @@
 #include "clang/Sema/Ownership.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Registry.h"
 #include "llvm/Support/VersionTuple.h"
@@ -783,7 +782,7 @@
   friend class AttributeFactory;
   friend class ParsedAttributes;
   AttributeFactory &Factory;
-  llvm::TinyPtrVector<ParsedAttr *> Attrs;
+  llvm::SmallVector<ParsedAttr *> Attrs;
 
   void *allocate(size_t size) {
     return Factory.allocate(size);
@@ -908,7 +907,7 @@
 };
 
 class ParsedAttributesView {
-  using VecTy = llvm::TinyPtrVector<ParsedAttr *>;
+  using VecTy = llvm::SmallVector<ParsedAttr *>;
   using SizeType = decltype(std::declval<VecTy>().size());
 
 public:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128097.438034.patch
Type: text/x-patch
Size: 975 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220617/5242dae8/attachment-0001.bin>