[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]

lebedev.ri abandoned this revision.
lebedev.ri added a comment.

In https://reviews.llvm.org/D54514#1305100, @courbet wrote:

> 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


Cool.

> (not in the current form though, but when everything is moved to a separate struct).

Abstractions are good :)


Repository:
  rL LLVM

https://reviews.llvm.org/D54514