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

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

In https://reviews.llvm.org/D54514#1303407, @lebedev.ri wrote:

> 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.


I think I have a slight preference for the other commit because of the vector vs deque (not in the current form though, but when everything is moved to a separate struct).


Repository:
  rL LLVM

https://reviews.llvm.org/D54514