[PATCH] D54442: [llvm-exegesis] Optimize ToProcess in dbScan

Clement Courbet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 21 02:21:29 PST 2018


courbet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/Clustering.cpp:120
     // Process P's neighbors.
-    llvm::SetVector<size_t, std::deque<size_t>> ToProcess;
-    ToProcess.insert(Neighbors.begin(), Neighbors.end());
-    while (!ToProcess.empty()) {
+    size_t Tail = 0;
+    for (size_t Q : Neighbors)
----------------
All of this makes the dbscan implementation harder to follow. Please move all this to a separate struct:

```
// This is long-lived.
struct WorkSetStorage {
  public:
   WorkSetStorage(size_t NumPoints) : ToProcess(NumPoints), Added(NumPoints) {}

   void setProcessed(size_t P) { Added[P] = 1; }

  private:
   friend class WorkSet;
   std::vector<size_t> ToProcess;
   std::vector<char> Added;
};

// This is short-lived and replaces  llvm::SetVector<size_t, std::deque<size_t>> ToProcess;
class WorkSet {
  public:
   WorkSet(WorkSetStorage& Storage) : Storage(Storage), Head(0), Tail(0) {}

   // Inserts a point if not already processed.
   void insert(size_t P);

   // returns the first element from the work set and pops it.
   size_t pop();

  private:
   WorkSetStorage& Storage;
   size_t Head;
   size_t Tail;
};
```


================
Comment at: tools/llvm-exegesis/lib/Clustering.cpp:141
       // And extend to the neighbors of Q if the region is dense enough.
-      rangeQuery(Q, Neighbors);
-      if (Neighbors.size() + 1 >= MinPts) {
-        ToProcess.insert(Neighbors.begin(), Neighbors.end());
-      }
+      if (Neighbors.size() + 1 >= MinPts)
+        for (size_t P : Neighbors)
----------------
did you just remove the `rangeQuery()` call on `Q` ?  You have the neighbours from `P` here. 


Repository:
  rL LLVM

https://reviews.llvm.org/D54442





More information about the llvm-commits mailing list