@mkrzewic : Could you add some more explanation of what is the target use here? Is it to support shared memory transport in DPL? Or being able to adopt another vector or the like? Thanks.

@sawenzel I updated the description. The point is to transparently be able to take advantage of shared memory/ofi transports without copying data while just using standard containers (currently only std::vector and std::string would make sense afaik, in the future - who knows :)). So if you use an o2::pmr::vector<int>(outputChannelAllocator) - it gives you all the benefits of the container (e.g. dynamic size, move semantics) in the most efficient way supported by the transport.

also: when developing new containers we would only have to stick to the STL container requirements making the development standards-compliant and at the same time be "message-efficient" with no extra effort.

this PR does not expose allocators (any more than they already were), see
o2::pmr::vector<T> makeVector(const Output& spec, Args&&... args)

as for the ownership - modern C++ practice is to keep ownership with the data since move semantics allow us to do this efficiently - this way we don't have to have magical entities to manage resource lifetimes - so in that respect I cannot disagree more with your statement.
Actually - the resource is managed by the framework that created it - FairMQ - since somewhere buried inside the (invisible) allocator there lives a FairMQMessagePtr.

You were exposing the allocator in the user code, as in "user has to know the allocator type", as far as I remember. This is indeed not the case anymore and using something like the o2::pmr::vector is indeed the direction I would prefer.

The advantage of having an o2::ptr<C<T>> or o2::ref<C<T>> (or whatever, the point is to have a wrapper<C<T>> kind of class) rather than an o2::pmr::container<T> is that the code is actually general enough you could use it for anything which is an AllocatorAwareContainer (with extra checks once we get to C++20 Concepts).

The reason for having the Framework / DPL to create the object, rather than doing it in vacuum is that there is no chance to mistake and create the wrong type of vector.

To me:

auto r = ctx.outputs().makeVector<Clusters>(Output("TPC/TRACKS/0"), 10);

is better than:

auto v = o2::vector<Clusters>{10};
auto r = ctx.outputs().adoptVector(Output("TPC/TRACKS/0"), v);

You expose less of the details and you keep everything in one place. If you really do not like the ctx.outputs(). we could hide that behind a static std::makeVector<> but when we discussed this the feedback was that people liked to see the ctx.outputs() as it makes clear you are creating something outgoing.

Apart from that, which we can keep for a later discussion, I would drop the pmr namespace from o2::pmr::vector since we do not have and we will not have any o2::vector to differentiate from (i.e. an o2::vector is the vector to use if you want to exchange it using o2).

You were exposing the allocator in the user code, as in "user has to know the allocator type", as far as I remember. This is indeed not the case anymore and using something like the o2::pmr::vector is indeed the direction I would prefer.

it is the same, just different wording, user does have to know where the data is supposed to be going. Whether you call it allocator or output channel, I don't care, things can always be aliased with some dumbed down name.

The advantage of having an o2::ptr<C<T>> or o2::ref<C<T>> (or whatever, the point is to have a wrapper<C<T>> kind of class) rather than an o2::pmr::container<T> is that the code is actually general enough you could use it for anything which is an AllocatorAwareContainer (with extra checks once we get to C++20 Concepts).

I think this is superfluous, an allocator aware container already IS a wrapper around data, and has everything that is needed for efficient data transfers. No need to hide everything from users.

The reason for having the Framework / DPL to create the object, rather than doing it in vacuum is that there is no chance to mistake and create the wrong type of vector.

To me:

auto r = ctx.outputs().makeVector<Clusters>(Output("TPC/TRACKS/0"), 10);

is better than:

auto v = o2::vector<Clusters>{10};
auto r = ctx.outputs().adoptVector(Output("TPC/TRACKS/0"), v);

i'm not suggesting to do it in vacuum, just suggesting a more natural syntax (see previous post):
auto v = o2::vector<Clusters>{ctx.outputs().getTransportAllocator(), Args...} //make a vector suitable for output

this is just plain C++, no need to invent some additional syntax and logic for it.
In the pmr implementation the underlying message is managed by the container itself - so if you are happy with your data you return/move/send/whatever at the end, otherwise you drop it. No need to keep some state (the message) in the framework. So yes, you have to call something at the end (you have adoptVector, but I would just call it Send() or something). More explicit and more intuitive since it reflects the ownership. Also exception safety is ensured by simple scoping and clear(standard) ownership semantics.

You expose less of the details and you keep everything in one place. If you really do not like the ctx.outputs(). we could hide that behind a static std::makeVector<> but when we discussed this the feedback was that people liked to see the ctx.outputs() as it makes clear you are creating something outgoing.

i have no problem with ctx.outputs() being explicitly visible - other than the confusing naming (plural suggests you get some collection of outputs, but you get a DataAllocator (i think), all that is hidden by everything is auto, so hard to actually find the thing you are getting)

Apart from that, which we can keep for a later discussion, I would drop the pmr namespace from o2::pmr::vector since we do not have and we will not have any o2::vector to differentiate from (i.e. an o2::vector is the vector to use if you want to exchange it using o2).

I have no strong preference here, but I think there is no need to hide the fact that the vector is a pmr vector. Plus it would make it easy to switch to std::pmr::vector when that gets implemented in the standard libraries.

now I come to think of it: maybe the make* methods that return something managed (like the ones that call adopt(...) internally) should return by pointer, not by value (to avoid dtors being called on exception) and not by reference (confusing since value semantics suggest ownership).
The makeVector(...) I'm suggesting returns by value and it is then clear - the thing you get is self contained, you own it until you move it away. To avoid confusion maybe a different name should be chosen to avoid ambiguity with the other make* methods.

No, there is a huge difference semantically by having an allocator, an output channel or anything related to transport layer and specifying an output datatype, which does not know anything about how data gets transported to the rest of the consumers.

The example you have:

auto v = o2::vector<Clusters>{ctx.outputs().getTransportAllocator(), Args...}

will not be enough for the case in which you have multiple outputs or multiple transports. If at lower level as an implementation detail we want to use pmr and similar, fine, in the DPL the design tradeoff is:

• Users specify inputs in terms of O2 Data Model descriptors.
• Users create output in terms of the O2 Data Model descriptors.
• No knowledge is needed about the underneath transport and how and how many devices are connected using which technology.

These requirements are there since the beginning and I think they are turning out to be a good idea. I am happy to improve the API and remove confusions, but I think those principles should remain intact.

Regarding the naming of o2::pmr::vector vs o2::vector I do prefer o2::vector so if there is no strong objection, I think it would better to use the latter. I refuse to believe people will want to know about polymorphic memory resources. ;-)

The example you have:

auto v = o2::vector<Clusters>{ctx.outputs().getTransportAllocator(), Args...}

will not be enough for the case in which you have multiple outputs or multiple transports.

of course it is enough:

auto v = o2::vector<Clusters>{ctx.outputs().getTransportAllocator(Output("TPC/TRACKS/0")), Args...}

something like that will take care of selecting the proper one. We should probably also avoid specifying the output by string since it is supposed to be predeclared already. Also note there is no need to specify the size since it will either resize itself or user calls reserve() - it is a std::vector after all.

If at lower level as an implementation detail we want to use pmr and similar, fine, in the DPL the design tradeoff is:

• Users specify inputs in terms of O2 Data Model descriptors.
• Users create output in terms of the O2 Data Model descriptors.
• No knowledge is needed about the underneath transport and how and how many devices are connected using which technology.

These requirements are there since the beginning and I think they are turning out to be a good idea. I am happy to improve the API and remove confusions, but I think those principles should remain intact.

sure, nothing wrong with it, I don't see how my proposal breaks it. I'll reshuffle the code a bit to make it a bit more clear.

Regarding the naming of o2::pmr::vector vs o2::vector I do prefer o2::vector so if there is no strong objection, I think it would better to use the latter. I refuse to believe people will want to know about polymorphic memory resources. ;-)

should be possible, but i see a funny interference with std::vector after changing that - in some cases o2::vector gets precedence in implementation files which use "vector", even after a "using std::vector" statement at the beginning...

Regarding your example, I still prefer:

auto v = ctx.outputs().makeVector<Cluster>{Output("TPC/TRACKS/0")), Args...}
// or:
auto v = ctx.outputs().make<o2::vector<Cluster>>{Output("TPC/TRACKS/0")), Args...}
// or:
o2::ref<std::vector<Cluster>> v = ctx.outputs().make<std::vector<Cluster>>{Output("TPC/TRACKS/0")), Args...}

over:

auto v = o2::vector<Cluster>{ctx.outputs().getTransportAllocator(Output("TPC/TRACKS/0")), Args...}

as it is more symmetric with all the cases which we will have which do not support polymorphic allocators (e.g. histograms, boost serialised objects as implemented right now) or which have different mechanisms (e.g. arrow and it's pool_allocator business). How would you treat those cases?

just to answer you quickly: i'm not aiming at full symmetry, I don't believe it is quite possible and I would not like to be limited to only one possibility. As for the other cases: I don't know ATM, but i'll give it some thought:)
This PR however only contains the "symmetric" case: a DataAllocator::make method. The "manual construction" method is simply a fact of life and it should be possible to use it if desired. The "make" method is just syntactic sugar that wraps explicit construction.

IMHO, esposing allocators is as bad as an idea as it can possibly get. Data own by the framework should be created via the framework.

What do you mean exactly by "created"?

Assuming you mean "allocated", then Mikolajs contribution is exactly enforcing the property you stated above. If the framework does not expose its allocator, then the stated property will deny moving data to the framework, or am I missing anything? That is at least the feature I recognize Mikolaj wants to add here?

@dennisklein In an entity-component-system system, the system should be used to create entities and components. They should not come to life autonomously. In this case, a method of DPL should be used to construct an object, allocated with whatever policy (which is really an implementation detail and should not be exposed) and return it to the user. The user should not care / know how the object comes into life and the object itself should not expose how it comes into life or who it belongs to. Ideally I would not even expose the fact that the container is a std::vector (which requires guarantees on the contiguousness of the memory region) but I would expose an iterator pair / range, albeit that's a different story.

@mkrzewic given it's syntactic sugar, why not having symmetric sugar and have a specialisation which matches:

  o2::vector<T> make<o2::vector<T>>(const Output& spec, Args&&... args)

isn't what you propose a simply a more verbose and implementation details exposing version of the same thing? I am not against breaking symmetry (we do it actually for pointers and everything else already), but why deliberately breaking the symmetry?

@ktf makeVector looks how it looks like (i.e. slightly differently) because your make<> templates consistently return by reference - which is in my opinion dangerous since the compiler will not warn you if you catch by value - you get a local copy and will not be working with the entity that was registered internally...
Exposing iterators/ranges is a fantastic device, but only works for consumers of read-only data. When producing we don't know how big a range is going to be so we need access to the actual container and we need to actually control lifetime ourselves - my preference is to do it by using scoped handles like unique_ptr, std::vector etc... I think modern C++ gives us devices which render the older patterns less useful in their pure form - isn't an entity-component system largely a lifetime management system that was needed because it used to be hard to deal with object lifetime management?

isn't an entity-component system largely a lifetime management system that was needed because it used to be hard to deal with object lifetime management?

Nono, that's called ROOT. Or in other words: For every type T have another type TManager which is a singleton owning container of all the objects of T. :-P

Or in modern ROOT: TManager is a thread local singleton.

yes, I agree with your concern, this is why I was proposing to have an o2::observer_ptr<T> or whatever you want to call it to avoid such a case. Maybe I should retake on the exercise at some point.

Well, nevermind..

I squashed everything together for an easier read and added observer_ptr (could not find anything like that in boost), still not sure what to do with the makeVector() method...

it seems this is stuck. Since a large part of this PR is actually not DPL related and contains a long needed fix of the data header (like the removal of Stack::freefn - which has to go away) I could move the DPL related part to another PR, but then the author of DPLmerger (@gabrielefronze ) will have to fix or remove that code.

removing [WIP], there is not much that can be changed here on short notice. This PR fixes a number of long standing issues related to the data model and the remaining concerns can be resolved in the future if needed.

ok, i'll reshuffle. closing in favour of #1581 and following.