[PATCH] D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs

Mon Jan 20 08:13:48 PST 2020

krisb added a comment.

In D72797#1829031 <https://reviews.llvm.org/D72797#1829031>, @djtodoro wrote:

> > So, this increases number of 'total vars' because these 'a' and 'b' variables counted twice.
>
> I see... Can we do something to avoid the additional calculation? Somehow to recognize we already observed the `a` and `b`, although they have different DIE ID.
>
> > I also was a bit optimistic about how this patch(es) affects the performance of the statistics, it actually became two times slower than before the patches.
>
> What patch from the stack adds the overhead? This one?

Yeah, this one mostly causes the overhead. 
Actually, adding declaration info for ID is only necessary for functions, global variables, and members (because we need to handle static/inline functions/variables, and don't respect namespaces, classes, and structures). For local variables and parameters, Prefix + Name should be enough to distinguish all the cases (because their prefix already guaranteed uniqueness, for parameters the prefix is going to be added in D73003 <https://reviews.llvm.org/D73003>). 
This approach shows better results:

               |   "source   |      baseline(master)    |         patched          |
  Binary name  |  variables" | "Availability" | time(s) | "Availability" | time(s) |
  ----------------------------------------------------------------------------------
  opt          |    637923   |         214%   |   1.644 |         100%   |   2.47  |
  lli          |    362669   |         177%   |   0.898 |          99%   |   1.346 |
  diagtool     |    459671   |         184%   |   1.399 |         100%   |   2.103 |
  arcmt-test   |    572726   |         180%   |   1.665 |         100%   |   2.53  |
  llc          |    527603   |         200%   |   1.431 |         100%   |   2.282 |
  clang-3.4    |   1445548   |         208%   |   4.33  |         100%   |   6.413 |
  llvm-lto     |    629415   |         208%   |   1.652 |         100%   |   2.666 |
  clang-check  |    621696   |         195%   |   2.108 |         100%   |   3.302 |
  clang-format |    470768   |         184%   |   1.429 |          99%   |   2.154 |
  llvm-c-test  |    478700   |         200%   |   1.302 |         100%   |   2.033 |

It also reduces the overhead of this patch to up to 61% (instead of up to 100% previously).
With this approach, we have a really small amount of 'collisions' but still have some.
I'll update the patch when I find the reason for the 'collisions' left.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72797/new/

https://reviews.llvm.org/D72797