diff options
| author | Roman Lebedev <lebedev.ri@gmail.com> | 2018-11-19 13:28:09 +0000 |
|---|---|---|
| committer | Roman Lebedev <lebedev.ri@gmail.com> | 2018-11-19 13:28:09 +0000 |
| commit | 0b4b512826c7331542348706d7374632c6ea45a0 (patch) | |
| tree | 269d91b3a220fc2c95dc02edff9230cfd92b7502 /llvm/tools | |
| parent | b879df24c9fca06d57123822d2cb58cb32d30988 (diff) | |
| download | bcm5719-llvm-0b4b512826c7331542348706d7374632c6ea45a0.tar.gz bcm5719-llvm-0b4b512826c7331542348706d7374632c6ea45a0.zip | |
[llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::SetVector<> instead of ILLEGAL std::unordered_set<>
Summary:
Test data: 500kLOC of benchmark.yaml, 23Mb. (that is a subset of the actual uops benchmark i was trying to analyze!)
Old time:
```
$ time ./bin/llvm-exegesis -mode=analysis -analysis-epsilon=100000 -benchmarks-file=/tmp/benchmarks.yaml -analysis-inconsistencies-output-file=/tmp/clusters.html &> /dev/null
real 0m24.884s
user 0m24.099s
sys 0m0.785s
```
New time:
```
$ time ./bin/llvm-exegesis -mode=analysis -analysis-epsilon=100000 -benchmarks-file=/tmp/benchmarks.yaml -analysis-inconsistencies-output-file=/tmp/clusters.html &> /dev/null
real 0m10.469s
user 0m9.797s
sys 0m0.672s
```
So -60%. And that isn't the good bit yet.
Old:
* calls to allocation functions: 106560180 (yes, 107 *million* allocations.)
* bytes allocated in total (ignoring deallocations): 12.17 GB
New:
* calls to allocation functions: 3347676 (-96.86%) (just 3 mil)
* bytes allocated in total (ignoring deallocations): 10.52 GB (~2GB less)
---
Two points i want to raise:
* `std::unordered_set<>` should not have been used there in the first place.
It is banned by the https://llvm.org/docs/ProgrammersManual.html#other-set-like-container-options
* There is no tests, so i'm not fully sure this is correct.
Since it was unordered set, i guess there are zero restrictions on the order, and anything will be ok?
* I tried other containers suggested in https://llvm.org/docs/ProgrammersManual.html#set-like-containers-std-set-smallset-setvector-etc,
this `llvm::SetVector<>` seems to be best here.
Reviewers: courbet, MaskRay, RKSimon, gchatelet, john.brawn
Reviewed By: courbet
Subscribers: kristina, bobsayshilol, tschuett, llvm-commits
Differential Revision: https://reviews.llvm.org/D54381
llvm-svn: 347197
Diffstat (limited to 'llvm/tools')
| -rw-r--r-- | llvm/tools/llvm-exegesis/lib/Clustering.cpp | 7 |
1 files changed, 4 insertions, 3 deletions
diff --git a/llvm/tools/llvm-exegesis/lib/Clustering.cpp b/llvm/tools/llvm-exegesis/lib/Clustering.cpp index 761629167bb..f61bfbc3160 100644 --- a/llvm/tools/llvm-exegesis/lib/Clustering.cpp +++ b/llvm/tools/llvm-exegesis/lib/Clustering.cpp @@ -8,8 +8,8 @@ //===----------------------------------------------------------------------===// #include "Clustering.h" +#include "llvm/ADT/SetVector.h" #include <string> -#include <unordered_set> namespace llvm { namespace exegesis { @@ -120,11 +120,12 @@ void InstructionBenchmarkClustering::dbScan(const size_t MinPts) { CurrentCluster.PointIndices.push_back(P); // Process P's neighbors. - std::unordered_set<size_t> ToProcess(Neighbors.begin(), Neighbors.end()); + llvm::SetVector<size_t> ToProcess; + ToProcess.insert(Neighbors.begin(), Neighbors.end()); while (!ToProcess.empty()) { // Retrieve a point from the set. const size_t Q = *ToProcess.begin(); - ToProcess.erase(Q); + ToProcess.erase(ToProcess.begin()); if (ClusterIdForPoint_[Q].isNoise()) { // Change noise point to border point. |

