Support for dynamic size types

elfenpiff commented 2 years ago

Brief feature description

At the moment it is impossible to send types in a zero copy manner which have a size that is only known at runtime. The current approach is to use a std::vector or similar container and copy/serialize the content into an untyped message via the untyped publisher. I propose to adjust our containers cxx::vector, cxx::string and maybe later cxx::(forward)list so that they support dynamic size instead of only compile time size.

Detailed information

The cxx containers should be extended so that a custom allocator can be attached which allocates memory inside of the shared memory.

The extension should be done in a manner so that the capacity template parameter still works, e.g. cxx::vector<int, 10> bla; should create a vector with a stack allocator of size 10.
The allocator should not effect function developers which are using such containers as input parameters. This would be bad: void myFunction(cxx::vector<int, SpecificAllocator> & v); we want void myFunction(cxx::vector<int> &v) without providing the allocator type.
Since the containers are zero copy containers the capacity has to be reserved first before filling them, otherwise intransparent copies occur due to resizing allocated memory which cannot be done in a contiguous way.
```
cxx::vector<int, SomeAllocator> myVec;
myVec.reserve(10); // only after reserve the container can be filled with elements, can be called only once
myVec.emplace_back(1234);
```
see also previous discussion in #594

Usage example

struct MyData {
  MyData(Allocator & allocator) 
    : vecData(allocator), text(allocator) {}

  int data;
  cxx::vector<int, Allocator> vecData;
  cxx::string<Allocator> text;
};

auto sample = publisher.loan();
sample.value()->vecData.reserve(1234); // reserve 1234 elements in vector, can be called only once
sample.value()->vecData.emplace_back(4249);

sample.value()->text.reserve(12);
sample.value()->text.assign("hello world");
sample.publish();

Tasks

[ ] Write design document
[ ] Implement allocator which does not need to be configured, (see mempool config of roudi)
[ ] Write example to demonstrate C++ use case with cxx::vector and cxx::string
[ ] Write example to demonstrate in C++ a C use case

struct MySample {
  void * data;
  int dataLength;
};

// use instead
struct MySample {
  cxx::vector<uint8_t> data;
};

Further Information

See #488 containers with a size of zero.

elfenpiff commented 2 years ago

@Indra5196 @rex-schilasky this is maybe interesting for you. Especially the Usage example, would you be happy with such an API or do you see some problems?

mossmaurice commented 2 years ago

Nice feature & ideas!

Few questions that popped up regarding 74f6d10:

What happens if the user is using the ShmAllocator but has no shared memory mapped to virtual address space?
I assume the ShmAllocator would be part of iceoryx_posh and not iceoryx_hoofs, correct?
The allocator should not effect function developers .. without providing the allocator type
- Can you elaborate on how you want to solve it?

You can also answer them later in the design PR or maybe it's also a good idea to put this onto the agenda of tomorrow's iceoryx developer meetup.

@ZhenshengLee might also be interesting for you.

elfenpiff commented 2 years ago

@mossmaurice I am currently in the middle of writing the design document and still working on this whole concept. I would suggest that we handle those questions in the upcoming design document PR. I hope that it will pop up at the end of the week.

rex-schilasky commented 2 years ago

@elfenpiff in general it sounds very promising that iceoryx can handle dynamic size types. Would that fix the issue 'fixed-size types only' for the ROS2 cyclonedds/iceoryx setup ? For eCAL the current iceoryx write logic is very primitive and looks like this

ecal/core/src/readwrite/ecal_writer_iceoryx.cpp

size_t CDataWriterSHM::Send(const SWriterData& data_)
  {
    if (!m_publisher) return 0;
    size_t ret(0);

    uint32_t payload_size(static_cast<uint32_t>(data_.len));
    uint32_t payload_alignment(static_cast<uint32_t>(alignof(void*)));
    uint32_t header_size(static_cast<uint32_t>(sizeof(data_)));
    uint32_t header_alignment(static_cast<uint32_t>(alignof(SWriterData)));

    m_publisher->loan(payload_size, payload_alignment, header_size, header_alignment)
      .and_then([&](auto& userPayload) {
        // loan successful
        // copy payload header
        std::memcpy(iox::mepoo::ChunkHeader::fromUserPayload(userPayload)->userHeader(), &data_, sizeof(SWriterData));
        // copy payload data
        std::memcpy(static_cast<char*>(userPayload), data_.buf, data_.len);
        // publish all
        m_publisher->publish(userPayload);
        ret = data_.len;
      }).or_else([&](auto& error) {
        // loan failed
        std::stringstream ss;
        ss << "CDataWriterSHM::Send(): Loan of iceoryx chunk failed ! Error code: " << static_cast<uint64_t>(error);
        Logging::Log(log_level_fatal, ss.str());
      });

    return ret;
  }

where SWriterData contains the header and the payload information

ecal/core/src/readwrite/ecal_writer_base.h

struct SWriterData
    {
      const void*  buf;
      size_t       len;
      long long    id;
      long long    clock;
      size_t       hash;
      long long    time;
    };

so the SWriterData struct is the interface from eCAL to the writers (UDP/SHM). I have two questions ..

Is this approach in general iceoryx like from performance perspective ?
Will this feature give some improvement here ?

elfenpiff commented 2 years ago

@rex-schilasky

It will be iceoryx like from the performance perspective! You get dynamic sized types and we can still guarantee no copies.
From the first look it seems that the biggest performance hit in this piece of code is the line where you have to copy SWriterData::buf into an iceoryx chunk with std::memcpy(static_cast<char*>(userPayload), data_.buf, data_.len);

If you could modify SWriterData so that it looks like

struct SWriterData
    {
      cxx::vector<uint8_t> buf;
      //...

And use it like:

auto sample = m_publisher->loan();
sample->buf.reserve(data_.len);
void* data = sample.data();

and write directly into data then you could avoid this memcpy which should provide you a nice performance boost.

I think somewhere in your code you allocate the memory required for SWriterData and this allocation has to be replaced with publisher->loan(); sample->buf.reserve(data.len);. The send method then gets the sample, sets the custom header and calls sample.publish().

When the feature is implemented there will be an example showing this in greater detail. I added it as task.

rex-schilasky commented 2 years ago

@elfenpiff thank you for your explanation. Yes the payload memcpy hurts mostly. To use a cxx::vector<uint8_t> inside the SWriterData struct would be perfect for sure. Currently we can not do so because this would impact many eCAL communication layer (all based on that struct) and finally the user API that is providing this address. But we keep this in mind.

ZhenshengLee commented 2 years ago

Nice feature & ideas!

Few questions that popped up regarding 74f6d10:

@ZhenshengLee might also be interesting for you.

@mossmaurice long time no see!

As my repo ros2_shm_msgs finished recently, I got stars and the feedback from https://github.com/eProsima/Fast-DDS/discussions/2773#discussioncomment-3078085

pre-defining data of various sizes (1m~8m) would cause a huge waste for my application,

So, this feature will give a chance to resolve this.

Grabber commented 1 year ago

Some unrelated experiments I did on passing a std::vector as a void pointer: https://gist.github.com/Grabber/5082952fdb161713416bcc5d244d5c16

eclipse-iceoryx / iceoryx