[PATCH] D54514: [llvm-exegesis] InstructionBenchmarkClustering::dbScan(): use manual std::deque<size_t> + std::vector<char> instead of SetVector.

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 19 13:21:29 PST 2018


lebedev.ri added a comment.

In https://reviews.llvm.org/D54514#1303148, @MaskRay wrote:

> In https://reviews.llvm.org/D54514#1303127, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D54514#1303114, @MaskRay wrote:
> >
> > > Anyway, I think this is superseded by https://reviews.llvm.org/D54442
> >
> >
> > I looked at that, and i think it does at least four things at once:
> >
> > 1. Exactly what this diff does, `llvm::SetVector<>` -> `std::{CONTAINER}<size_t> Workqueue` + `std::{CONTAINER}<size_t> Set`.
>




>> 2. `std::deque<size_t> Workqueue` -> `std::vector<size_t>` with manual begin/end tracking. FIXME: how does this affect the capacity?
> 
> The queue does not need to use `std::queue`. It can be pre-allocated and use manual begin/end tracking. This does not affect the capacity (<= N).

Aha, ok, then that your change looks like a good follow-up.

>> 3. Not removing from the `Set` after processing.
>> 4. `assert` instead of `continue`

I'll leave it up to @courbet.
I'm just not super happy these 'assortment' commits, with multiple **separable** changes.


Repository:
  rL LLVM

https://reviews.llvm.org/D54514





More information about the llvm-commits mailing list