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

Ben via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 11 05:54:04 PST 2018


bobsayshilol added a comment.

I could be missing something, but I don't understand why `ToProcess` needs to be a set-like container since we're erasing elements as we go (ie the erased elements won't be duplicate checked on next insertion). We skip any that have been previously processed in the inner loop too, which seems like it's doing the same work the set would be doing.

The `isNoise()` check also looks odd, since if `CurrentCluster.Id` has id `kNoise` then it could push the same index into `CurrentCluster.PointIndices` an unspecified number of times depending on the order that the elements are inserted and removed from `ToProcess`, but if `CurrentCluster.Id` can't be `kNoise` then that's not relevant.

>From the docs:

> SetVector is an adapter class that defaults to using std::vector

so calling erase on the first element isn't going to be terribly efficient either.


Repository:
  rL LLVM

https://reviews.llvm.org/D54381





More information about the llvm-commits mailing list