[llvm] r349136 - [llvm-exegesis] Optimize ToProcess in dbScan

[Simon Pilgrim via llvm-commits]

Its also causing buildbot assertions:

http://lab.llvm.org:8011/builders/clang-cmake-x86_64-avx2-linux/builds/7402/steps/ninja%20check%201/logs/stdio

On 14/12/2018 08:57, Roman Lebedev via llvm-commits wrote:
> I'm sorry, was the LGTM given off-line?
> I don't think this got reviewed otherwise.
> Both my's and courbet's feedback was that it is kinda ugly without abstractions,
> and i also additionally did not like that it does contain more separate changes.
>
> Roman.
>
> On Fri, Dec 14, 2018 at 11:30 AM Fangrui Song via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: maskray
>> Date: Fri Dec 14 00:27:35 2018
>> New Revision: 349136
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=349136&view=rev
>> Log:
>> [llvm-exegesis] Optimize ToProcess in dbScan
>>
>> Summary:
>> Use `vector<char> Added + vector<size_t> ToProcess` to replace `SetVector ToProcess`
>>
>> We also check `Added[P]` to enqueueing a point more than once, which
>> also saves us a `ClusterIdForPoint_[Q].isUndef()` check.
>>
>> Reviewers: courbet, RKSimon, gchatelet, john.brawn, lebedev.ri
>>
>> Subscribers: tschuett, llvm-commits
>>
>> Differential Revision: https://reviews.llvm.org/D54442
>>
>> Modified:
>>      llvm/trunk/tools/llvm-exegesis/lib/Clustering.cpp
>>
>> Modified: llvm/trunk/tools/llvm-exegesis/lib/Clustering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-exegesis/lib/Clustering.cpp?rev=349136&r1=349135&r2=349136&view=diff
>> ==============================================================================
>> --- llvm/trunk/tools/llvm-exegesis/lib/Clustering.cpp (original)
>> +++ llvm/trunk/tools/llvm-exegesis/lib/Clustering.cpp Fri Dec 14 00:27:35 2018
>> @@ -8,7 +8,6 @@
>>   //===----------------------------------------------------------------------===//
>>
>>   #include "Clustering.h"
>> -#include "llvm/ADT/SetVector.h"
>>   #include "llvm/ADT/SmallVector.h"
>>   #include <string>
>>
>> @@ -92,8 +91,14 @@ llvm::Error InstructionBenchmarkClusteri
>>   }
>>
>>   void InstructionBenchmarkClustering::dbScan(const size_t MinPts) {
>> -  std::vector<size_t> Neighbors; // Persistent buffer to avoid allocs.
>> -  for (size_t P = 0, NumPoints = Points_.size(); P < NumPoints; ++P) {
>> +  const size_t NumPoints = Points_.size();
>> +
>> +  // Persistent buffers to avoid allocs.
>> +  std::vector<size_t> Neighbors;
>> +  std::vector<size_t> ToProcess(NumPoints);
>> +  std::vector<char> Added(NumPoints);
>> +
>> +  for (size_t P = 0; P < NumPoints; ++P) {
>>       if (!ClusterIdForPoint_[P].isUndef())
>>         continue; // Previously processed in inner loop.
>>       rangeQuery(P, Neighbors);
>> @@ -109,14 +114,18 @@ void InstructionBenchmarkClustering::dbS
>>       Cluster &CurrentCluster = Clusters_.back();
>>       ClusterIdForPoint_[P] = CurrentCluster.Id; /* Label initial point */
>>       CurrentCluster.PointIndices.push_back(P);
>> +    Added[P] = 1;
>>
>>       // 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)
>> +      if (!Added[Q]) {
>> +        ToProcess[Tail++] = P;
>> +        Added[Q] = 1;
>> +      }
>> +    for (size_t Head = 0; Head < Tail; ++Head) {
>>         // Retrieve a point from the set.
>> -      const size_t Q = *ToProcess.begin();
>> -      ToProcess.erase(ToProcess.begin());
>> +      size_t Q = ToProcess[Head];
>>
>>         if (ClusterIdForPoint_[Q].isNoise()) {
>>           // Change noise point to border point.
>> @@ -124,17 +133,18 @@ void InstructionBenchmarkClustering::dbS
>>           CurrentCluster.PointIndices.push_back(Q);
>>           continue;
>>         }
>> -      if (!ClusterIdForPoint_[Q].isUndef()) {
>> -        continue; // Previously processed.
>> -      }
>> +      assert(ClusterIdForPoint_[Q].isUndef());
>>         // Add Q to the current custer.
>>         ClusterIdForPoint_[Q] = CurrentCluster.Id;
>>         CurrentCluster.PointIndices.push_back(Q);
>>         // 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)
>> +          if (!Added[P]) {
>> +            ToProcess[Tail++] = P;
>> +            Added[P] = 1;
>> +          }
>>       }
>>     }
>>     // assert(Neighbors.capacity() == (Points_.size() - 1));
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits