Skip to content

Commit

Permalink
[llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::S…
Browse files Browse the repository at this point in the history
…etVector<> 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
  • Loading branch information
LebedevRI committed Nov 19, 2018
1 parent b879df2 commit 0b4b512
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions llvm/tools/llvm-exegesis/lib/Clustering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
//===----------------------------------------------------------------------===//

#include "Clustering.h"
#include "llvm/ADT/SetVector.h"
#include <string>
#include <unordered_set>

namespace llvm {
namespace exegesis {
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 0b4b512

Please sign in to comment.