[PATCH] D50214: Add inherited attributes before parsed attributes.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.


================
Comment at: lib/AST/DeclBase.cpp:854-859
+  auto I = Attrs.begin(), E = Attrs.end();
+  for (; I != E; ++I) {
+    if (!(*I)->isInherited())
+      break;
+  }
+  Attrs.insert(I, A);
----------------
aaron.ballman wrote:
> The unfortunate part about this is that inherited attributes are fairly common, so this extra searching may happen more often than we'd like. I wonder how bad it would be to keep track of the location within the list where the inherited attributes start. Then again, the list of attributes should be relatively short in most cases, so this search isn't likely to be too expensive.
> 
> Having some performance measurements might help decide this, too.
Timed clang-compilation using perf (`perf stat bin/clang llvm/unittests/IR/InstructionsTest.cpp ...`), before this patch (r339574):
```
       7647.415800      task-clock (msec)         #    0.983 CPUs utilized
               289      context-switches          #    0.038 K/sec
                26      cpu-migrations            #    0.003 K/sec
            86,494      page-faults               #    0.011 M/sec
    19,068,741,863      cycles                    #    2.493 GHz
    26,581,355,844      instructions              #    1.39  insn per cycle
     6,242,394,037      branches                  #  816.275 M/sec
       128,405,656      branch-misses             #    2.06% of all branches

       7.779848330 seconds time elapsed
```

With this patch:
```
       7679.173467      task-clock (msec)         #    0.987 CPUs utilized
               321      context-switches          #    0.042 K/sec
                23      cpu-migrations            #    0.003 K/sec
            86,071      page-faults               #    0.011 M/sec
    19,150,248,538      cycles                    #    2.494 GHz
    26,667,465,697      instructions              #    1.39  insn per cycle
     6,262,381,898      branches                  #  815.502 M/sec
       128,527,669      branch-misses             #    2.05% of all branches

       7.779477815 seconds time elapsed
```
(Intel(R) Xeon(R) Platinum 8180M CPU @ 2.50GHz)

The vector insert operation is also be `O(n)`. If the number of non-inherited is expected to be smaller, we can instead search for the first inherited attribute starting at the end of the vector. If we want to avoid the `O(n)` entirely, we need one list for inherited and another for non-inherited attributes.


Repository:
  rC Clang

https://reviews.llvm.org/D50214