Initial implementation of C api #915
Conversation
erer1243
force-pushed
the
c-interface
branch
from
bc232be
to
f621b2a
Compare
erer1243
force-pushed
the
c-interface
branch
from
4dbe2da
to
cf69f20
Compare
common/zmqconsumerstatetable.cpp
Outdated
| return; | ||
| } | ||
| } | ||
| std::lock_guard<std::mutex> lock(m_receivedQueueMutex); |
There was a problem hiding this comment.
I'm concerned about the performance impact of this change. The original code only locks when operating on m_receivedOperationQueue. This is because vkco.push_back(std::move(kco)) will copy a large amount of data and take a long time.
With this change, it may affect the ZMQ channel throughput.
There was a problem hiding this comment.
There are two race conditions in the code as-is if 2+ threads are running pops(): count can be invalidated, leading to auto& kco = m_receivedOperationQueue.front() giving a null reference; kco can also be popped and freed while still being pushed onto vkco, leading to use-after-free. Unfortunately we have to move the data while holding the lock.
There was a problem hiding this comment.
to avoid any potential perf issue. let's try this -
- define a new temporary queue, which will be empty.
- Lock the member queue and swap with the empty queue.
- pop all items from the temporary queue and push them all into the output queue.
the item 3, there is no lock in place, which should minimize the overall block/lock time.
There was a problem hiding this comment.
hope this solves the concern. @erer1243
There was a problem hiding this comment.
Riff's suggestion is a good idea. I think that will fix both sides of the issue.
| @@ -0,0 +1,34 @@ | |||
| #include <cstdlib> | |||
There was a problem hiding this comment.
Why create this c wrapper? Is there some issue to use swss-common lib with https://github.com/rust-lang/rust-bindgen?
There was a problem hiding this comment.
bindgen does not really help you call common C++. it only works well for C. It doesn't support types of undefined sizes (bool) and several common classes (vector, string). I created this C wrapper so that I can use bindgen
erer1243
force-pushed
the
c-interface
branch
from
cf69f20
to
8c8713c
Compare
erer1243
force-pushed
the
c-interface
branch
from
8c8713c
to
3dab797
Compare
assuming count stays valid and that no modifications happen between front() and pop() are classic check-then-act race conditions
erer1243
force-pushed
the
c-interface
branch
from
3dab797
to
74a5cc0
Compare
Implement a C interface to some of libswsscommon in support of sonic-dash-ha.
Related:
sonic-net/sonic-dash-ha#6
#921