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

[Ben via Phabricator via llvm-commits]

bobsayshilol added a comment.

In https://reviews.llvm.org/D54381#1294619, @lebedev.ri wrote:

> I think the usage of `Set` is correct.
>  We will, at most, fail to deduplicate one element (the one we just removed) out of `N` returned by `rangeQuery(Q)`.
>  But if we don't do any deduplication at all, then we will add all the `N` elements. And that sounds exponential.


Ah I meant with deduplication via something like:

  auto MiddleIt = ToProcess.insert(ToProcess.end(), Neighbors.begin(), Neighbors.end());
  std::inplace_merge(ToProcess.begin(), MiddleIt, ToProcess.end());
  std::erase(std::unique(ToProcess.begin(), ToProcess.end()), ToProcess.end());

when we're inserting elements, and then popping from the back of the vector at the beginning of the loop (since the iteration order for a `std::unordered_set` was undefined anyway). That does rely on the fact that `rangeQuery()` currently returns a sorted vector though.

In https://reviews.llvm.org/D54381#1294619, @lebedev.ri wrote:

> Here i only improve the choice of the data structure, and don't do any algorithm-changing decisions.


I don't think the above changes the algorithmic behaviour, but I could be wrong.

In https://reviews.llvm.org/D54381#1294619, @lebedev.ri wrote:

> Interesting question, but irrelevant for the patch at hand.


That's a valid point :)


Repository:
  rL LLVM

https://reviews.llvm.org/D54381