[PATCH] D91017: [BranchProbabilityInfo] Use SmallVector (NFC)

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 7 19:11:26 PST 2020


MaskRay added a comment.

In D91017#2381226 <https://reviews.llvm.org/D91017#2381226>, @kazu wrote:

> In D91017#2381145 <https://reviews.llvm.org/D91017#2381145>, @MaskRay wrote:
>
>> It is true that SmallVector is more convenient to use but this change also has potential to increase memory usage. Have you checked whether this can increase memory usage?
>
> The differences in memory consumption seem to be really small.
>
> I have invoked clang for three input files, three times each, and looked at `Maximum resident set size (kbytes)` with `/usr/bin/time -v`.
>
>   JumpThreading.ii
>   Without this patch: 280792 281016 281392
>   With this patch:    280736 281484 281568
>   
>   SROA.ii
>   Without this patch: 311820 312412 312488
>   With this patch:    311224 311356 311468
>   
>   LICM.ii
>   Without this patch: 346300 346380 348356
>   With this patch:    345528 348084 348564
>
> Under the hood, the difference should be a wash because:
>
>   DenseMap<std::pair<const BasicBlock *, unsigned>, BranchProbability>::value_type
>
> is 24 bytes in size, padded to 32 bytes, and
>
>   DenseMap<BasicBlock *, SmallVector<BranchProbability, 2>>::value_type
>
> is 32 bytes in size.
>
> If we assume that we have a function with 100 "if" statements with no switch statements at 50% occupancy of the hash table, we have:
>
>   Without this patch: 32 bytes/entry * 200 edges        / 50% occupancy = 12,800 bytes
>   With this patch:    32 bytes/entry * 100 basic blocks / 50% occupancy =  6,400 bytes

Thanks for the explanation!



================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:1098
+
+  auto &SrcProbs = I->second;
+  assert(SrcProbs.size() == Src->getTerminator()->getNumSuccessors() &&
----------------
`const std::vector<...> &` (specify the concrete type if unclear)


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:1120
 
+  auto &SrcProbs = SrcProbsIt->second;
+  assert(SrcProbs.size() == Src->getTerminator()->getNumSuccessors() &&
----------------
`const std::vector<...> &` (specify the concrete type if unclear)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91017



More information about the llvm-commits mailing list