Develop zerocopy #1053

TheMutta · 2024-07-01T10:52:42Z

This update leverages the new FD protocol in XLink to avoid unecessary copying, allowing for a zerocopy runtime on localhost.

This is allowed by the new memory class, SharedMemory, which instread of containing the data itself, like VectorMemory, it only holds the FD and the mapping.

…lper XLink functions for FDs

…r protocol

moratom

Thanks Filippo!

I left a small nitpicks.
You can run clangformat on the code with cmake --build build --target clangformat.

I think we should extend SharedMemory so it can be allocated on core side (conditionally for linux builds).
And to extend writingThread in DataQueue.cpp to send out frames with shared memory when possible.

To allow for fast transfer in both directions.

moratom · 2024-07-02T17:00:46Z

CMakeLists.txt

@@ -15,7 +15,7 @@ if(WIN32)
    set(DEPTHAI_CURL_USE_OPENSSL OFF)
 else()
    set(DEPTHAI_CURL_USE_SCHANNEL OFF)
-    set(DEPTHAI_CURL_USE_OPENSSL ON)


I think this can be removed.

moratom · 2024-07-02T17:02:33Z

examples/host_side/host_only_image.cpp

I think we can remove this one before the merge.

moratom · 2024-07-05T13:57:49Z

include/depthai/pipeline/datatype/ADatatype.hpp

@@ -6,6 +6,8 @@
 #include "depthai/utility/Memory.hpp"
 #include "depthai/utility/Serialization.hpp"
 #include "depthai/utility/VectorMemory.hpp"
+#include "depthai/utility/SharedMemory.hpp"


Not needed?

Yes, removed

moratom · 2024-07-05T13:58:25Z

include/depthai/pipeline/datatype/Buffer.hpp


    /**
     * @param data Moves data to internal buffer
     */
    void setData(std::vector<std::uint8_t>&& data);
+


Run clangformat target

moratom · 2024-07-05T14:03:46Z

include/depthai/utility/SharedMemory.hpp

+    SharedMemory() {
+	kind = MemoryKinds::MEMORY_KIND_SHARED_MEMORY;
+	fd = -1;
+    }


Does it make sense to allow this?

I think it would make more sense to only allow to pass in a size and allocate the shared memory.

Yeah probably useless, i removed it.

I think it would make more sense to only allow to pass in a size and allocate the shared memory.

Well, yes, that makes sense. So both passing the FD externally or automatically allocating with a shared memory buffer.

Also, i think it would be worth it looking into memfd_create(), since it is automatically closed when all references are released, which would be better than shm_open, which must be unlinked.

Please look into memfd_create() in that case so we can send data zero copy in the other direction as well then yeah.

It is pretty nice. It's like shm_open(), but better. You can do sealing and it has no need for unlinking.
The only issue would be name generation, which is the same for shm_open().
I'd wager we could use a static class variable to create a string like "shared_memory_" and then add the number of instances created. I think that should last us up until 2^64 instances :)

Oh btw, that constructor is actually needed because of the GstBufferMemory constructor in DepthAI device, as we set the FD in the constructor itself.

If we set it in the constructor, can we just initialize shared memory in the initializer list?

GstBufferMemory(int fd): SharedMemory(fd) {}...

Oh I see what you mean (checked the source).

Let's leave it then.

moratom · 2024-07-05T14:04:13Z

include/depthai/utility/SharedMemory.hpp

+    void setSize(std::size_t size) override {
+	ftruncate(fd, size);
+    }


What if size is bigger than current size?

Well, technically ftruncate extends it, but how that would interact with the various kinds of shared memory depends on the implementation

If it's for example memory created with shm_open, it will just expand it. If it's a DMA buffer, it depends on the implementation. For Gst, it seems to be the same as shm_open.
This behavior should be overridden by implementations if it's different.

However, we probably need to unmap and map the FD so that we can actually have a mapping of the correct size.

Makes sense, I though truncate only downsizes.

moratom · 2024-07-05T14:05:09Z

include/depthai/utility/VectorMemory.hpp

+    VectorMemory() { kind = MemoryKinds::MEMORY_KIND_VECTOR_MEMORY; }
+    VectorMemory(const std::vector<std::uint8_t>& d) : vector(std::move(d)) { kind = MemoryKinds::MEMORY_KIND_VECTOR_MEMORY; }
+    VectorMemory(std::vector<std::uint8_t>&& d) : vector(std::move(d)) { kind = MemoryKinds::MEMORY_KIND_VECTOR_MEMORY; }


We can remove the kind I think and use casts instead.

I think the kind is important, as for example in XLinkOut in DepthAI device, we need it to discern how to output. Unless we want to understand which it is by using sizeof(), which is imperfect, this system works better.

Can we use if(std::dynamic_ptr_cast<SharedMemory>(data))?

I didn't think about that...
That should work yes. I've tested it on a separate program and it works. I'm not sure about overhead, but it should be comparable anyway.

Seems like it doesn't work. I've reinstated the kind variables and they work now.

…setSize()

TheMutta · 2024-07-12T10:27:49Z

Branch is ready for merge

lnotspotl · 2024-07-12T10:48:42Z

There is a missing fd at the end of this line

TheMutta added 18 commits June 17, 2024 19:59

Added SharedMemory header

4a1f818

Finalized SharedMemory class

c36ab3b

Added differentiation between SharedMemory and VectorMemory, added he…

cc44859

…lper XLink functions for FDs

Replaced -1 fd with 0 fd

557abfd

This reverts commit 557abfd.

185186d

Made fd and mapping in SharedMemory protected instead of private

c4ff206

XLinkStream read helper functions for handling FDs

ad66010

Added SharedMemory support with FD in Buffer data structure

3c1d976

Added FD support in ImgFrame data structure

cf35d8c

Parsing fd in StreamMessageParser

a5016b6

Updated XLink version.

5019cca

Closing FDs when destructing SharedMemory

fd710a6

Added size parameters to stream write

1effc78

Passing FDs by value

88e86f8

Resolved conflict by reverting size changes.

0d4b57d

Added new max throughput example to test the efficency of the transfe…

979573a

…r protocol

Added new example in CMakeLists.txt

242d6ec

Disabled openssh that caused issues when cross compiling to RVC4

316c073

TheMutta requested a review from themarpe July 1, 2024 10:52

TheMutta self-assigned this Jul 1, 2024

themarpe requested review from moratom and removed request for themarpe July 1, 2024 16:19

Added X_LINK_TCP_IP_OR_LOCAL_SHDMEM

74441bd

moratom reviewed Jul 5, 2024

View reviewed changes

TheMutta added 6 commits July 5, 2024 18:15

Changes to removed unused constructor to SharedMemory and changes to …

587f9a2

…setSize()

Removed MemoryKinds

21e4649

Re-added empty constructor.

bc17b50

Added tcpshd enviroment variable

d8b81e8

Setting back connection in deviceInfo from connectionHandler

2efe453

Added memfd_create constructors.

cb9ac60

TheMutta added 6 commits July 11, 2024 12:33

Readded kinds.

9c4f05c

Added SharedMemory Fd writing in data queue

950dbd4

Updated XLink

48f664d

Clangformat

26fb39c

Removed example file

f99c0d8

reset OpenSSL compile flag

4560bb3

Merge branch 'v3_develop' into v3_develop_zerocopy

1a249f3

TheMutta merged commit 917e5ad into v3_develop Jul 12, 2024
1 of 11 checks passed

TheMutta deleted the v3_develop_zerocopy branch July 12, 2024 10:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop zerocopy #1053

Develop zerocopy #1053

TheMutta commented Jul 1, 2024

moratom left a comment

moratom Jul 2, 2024

moratom Jul 2, 2024

moratom Jul 5, 2024

TheMutta Jul 5, 2024

moratom Jul 5, 2024

moratom Jul 5, 2024

TheMutta Jul 5, 2024

moratom Jul 5, 2024

TheMutta Jul 5, 2024

TheMutta Jul 5, 2024

moratom Jul 5, 2024

moratom Jul 5, 2024

moratom Jul 5, 2024

TheMutta Jul 5, 2024

moratom Jul 5, 2024

moratom Jul 5, 2024

TheMutta Jul 5, 2024

moratom Jul 5, 2024

TheMutta Jul 5, 2024

TheMutta Jul 11, 2024

TheMutta commented Jul 12, 2024

lnotspotl commented Jul 12, 2024

Develop zerocopy #1053

Develop zerocopy #1053

Conversation

TheMutta commented Jul 1, 2024

moratom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheMutta commented Jul 12, 2024

lnotspotl commented Jul 12, 2024