[PATCH] D54381: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use llvm::SetVector<> instead of ILLEGAL std::unordered_set<>

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: courbet, MaskRay, RKSimon, gchatelet, john.brawn.
Herald added a subscriber: tschuett.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D54381

Files:
  tools/llvm-exegesis/lib/Clustering.cpp


Index: tools/llvm-exegesis/lib/Clustering.cpp
===================================================================
--- tools/llvm-exegesis/lib/Clustering.cpp
+++ 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 @@
     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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54381.173522.patch
Type: text/x-patch
Size: 1010 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181110/5860efea/attachment.bin>