[PATCH] D68816: [NFC] Replace a linked list in LiveDebugVariables pass with a DenseMap

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando updated this revision to Diff 224579.
Orlando edited the summary of this revision.
Orlando added a comment.

Addressed comments and fixed Summary (previously it referred to
LiveDebugValues.cpp instead of LiveDebugVariables.cpp).

@aprantl

> Not your code, but: add a FragmentInfo element?

As you point out in D66415 <https://reviews.llvm.org/D66415> we need to take care to track overlapping fragment
ranges too. This shouldn't be too hard but it makes more sense to me to rebase
D66415 <https://reviews.llvm.org/D66415> on this and work from there.

> How many elements does this have on average? Is a SmallDenseMap a win?

The following numbers are only anecdotal of course. I compiled two example
single file projects (A: ~100,000 loc, B: ~20,000 loc) and in both cases
roughly 90% of UserVarMap.size() were <= 64.

    --------------------------------------

  Project A: UserVarMap size
  --------------------------------------
  Size <= x | Num maps | % of total maps
  --------------------------------------
  0         | 697      | 50
  2         | 770      | 55.2
  4         | 797      | 57.2
  8         | 858      | 61.5
  16        | 969      | 69.5
  32        | 1109     | 79.6
  64        | 1221     | 87.6
  128       | 1311     | 94.0
  256       | 1361     | 97.6
  512       | 1379     | 98.9
  1024      | 1388     | 99.6
  
  --------------------------------------
  Project B: UserVarMap size
  --------------------------------------
  Size <= x | Num maps | % of total maps
  --------------------------------------
  0         | 296      | 50.3
  2         | 306      | 52.0
  4         | 333      | 56.6
  8         | 364      | 61.9
  16        | 397      | 67.5
  32        | 477      | 81.1
  64        | 530      | 90.1
  128       | 572      | 97.3
  256       | 582      | 99.0
  512       | 587	     | 99.8
  1024      | 588      | 100

I don't have any self-host build time stats for this yet but he default DenseMap
64 element alloc seems almost perfect for these examples. Assuming a
SmallDenseMap allocates space in the object (like SmallVector) and given that
LDVImpl seems to only ever be heap allocated, a SmallDenseMap<..., 64> would
likely give some (small) build time reduction. I'll add more info here
when I can get some self-host build times.


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

https://reviews.llvm.org/D68816

Files:
  llvm/lib/CodeGen/LiveDebugVariables.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68816.224579.patch
Type: text/x-patch
Size: 9756 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191011/a6ac0cc9/attachment.bin>