Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/true-zero-copy-7 #1082

Merged
merged 4 commits into from
Apr 25, 2023
Merged

feature/true-zero-copy-7 #1082

merged 4 commits into from
Apr 25, 2023

Conversation

rex-schilasky
Copy link
Contributor

@rex-schilasky rex-schilasky commented Apr 24, 2023

Pull request type

Please check the type of change your PR introduces:

  • Feature

What is the current behavior?
eCAL's current zero copy mode is realizing zero copy on the receiving side only. The sending (publishing) side still makes a single memory copy to provide a generic concept for all transport layers (zero copy and none zero copy capable ones).

What is the new behavior?
The new approach applies true zero copy for local publish/subscribe connections. It enables the direct modification of the memory file content without any additional copy action (zero copy).

The pictures shows two applications ecal_sample_latency_snd -z and ecal_sample_latency_rec running on Ubuntu 22.04, exchanging 32MB payloads in 3 µs.

Screenshot from 2023-04-11 17-20-48

Does this introduce a breaking change?

  • Yes

New CPublisher API functions added using the new eCAL::payload class.

Sample Usage

#include <ecal/ecal.h>
#include <ecal/ecal_publisher.h>

// binary payload writer class to allow zero copy memory operations
//
// the `WriteFull`and `WriteModified` calls may operate on the target memory file directly (zero copy mode)
class CBinaryPayloadWriter : public eCAL::CPayloadWriter
{
public:
  CBinaryPayloadWriter(size_t size_) : m_size(size_) {}

  // the provisioned memory is uninitialized ->
  // perform a full write operation
  bool WriteFull(void* buffer_, size_t size_) override
  {
    memset(buffer_, 42, size_);
    return true;
  };

  // the provisioned memory is initialized and contains the data from the last write operation ->
  // perform a partial write operation or just modify a few bytes here
  bool WriteModified(void* buffer_, size_t size_) override
  {
    size_t const write_idx((m_clock % 1024) % size_);
    char   const write_chr(m_clock % 10 + 48);
    static_cast<char*>(buffer_)[write_idx] = write_chr;
    m_clock++;
    return true;
  };

  // provide the size of the required memory (eCAL needs to allocate for you).
  size_t GetSize() override { return m_size; };

private:
  size_t m_size = 0;
  int    m_clock = 0;
};

// main entry
int main(int argc, char** argv)
{
  // initialize eCAL API
  eCAL::Initialize(argc, argv, "zero_copy_snd");

  // create payload
  CBinaryPayloadWriter binary_payload_writer(8 * 1024 * 1024);

  // create publisher
  eCAL::CPublisher pub("zero_copy_pub");

  // enable zero copy mode
  pub.ShmEnableZeroCopy(true);

  // enable handshake mode
  pub.ShmSetAcknowledgeTimeout(50 /* ms */);

  // send updates
  while (eCAL::Ok())
  {
    // send content
    pub.Send(binary_payload_writer);
  }

  // destroy publisher
  pub.Destroy();

  // finalize eCAL API
  eCAL::Finalize();

  return(0);
}

@rex-schilasky rex-schilasky changed the title true zero copy enhancement feature/true-zero-copy-7 Apr 24, 2023
@rex-schilasky rex-schilasky added the enhancement New feature or request label Apr 24, 2023
@rex-schilasky rex-schilasky added this to the eCAL 5.12 milestone Apr 24, 2023
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

ecal/core/src/readwrite/ecal_writer_inproc.h Outdated Show resolved Hide resolved
ecal/core/src/readwrite/ecal_writer_tcp.h Outdated Show resolved Hide resolved
ecal/core/src/readwrite/ecal_writer_tcp.h Show resolved Hide resolved
ecal/core/src/readwrite/ecal_writer_udp_mc.h Outdated Show resolved Hide resolved
class CPayloadWriter
{
public:
CPayloadWriter() = default;
Copy link
Contributor

@hannemn hannemn Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if itmake sense to explicitly define default contructors and assignment operator in an abstract base class as you cannot call them directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you still need them. Especially the assignment / move ones, since you NEED the virtual desctructor. If you don't have the others, the objects will not be movable!

Copy link
Contributor

@hannemn hannemn Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking about the contructors, not the destructor. In the end, the derived class is moveable as well as copyable by default even if the destructor of the abstract base class is marked as virtual. The compiler will anyways generate all default ctors and assigment operators if the user does not specify them on its own.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is false. This is why you have the rule of 5.
If you declare the destructor, copy assignment and copy constructor are defined, but move constructors / assigment ARE not. Thus your class will not be moveable.

@rex-schilasky rex-schilasky merged commit 3e33c89 into master Apr 25, 2023
13 checks passed
@rex-schilasky rex-schilasky deleted the feature/true-zero-copy-7 branch April 25, 2023 12:27
@chengguizi
Copy link

Very interesting development! Two questions:

  1. Will this be included in the next version release?
  2. Can it be already used with serialization layer, such as protobuf message or capnproto message. The example here shows only binary data

@rex-schilasky
Copy link
Contributor Author

rex-schilasky commented May 21, 2023

@chengguizi the zero-copy message transport feature will be part of the next 5.12 release.

The message protocols google protobuf and capnproto will be supported as well, flatbuffers currently not.

For these serialization formats the zero-copy feature would change the behavior that way that the message content is serialized in the opened memory file directly without creating an intermediate buffer. This new behavior may also have some overall performance drawbacks if your serialization call takes a long time and is so blocking the file for connected subscribers. But the good news is you can switch on/off this feature for every single publisher.

And yes by combining it with multiple write buffers you may get rid of the serialization file blocking.

@chengguizi
Copy link

Pretty much look forward, can't wait for 5.12 release!

@FlorianReimold FlorianReimold mentioned this pull request Jul 18, 2023
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants