ScatteredBuffer::IFragmentsObserver

[serges147]

• Fix unit tests broken by recent changes at CETL (std::bad_alloc at VariableLengthArray).

[pavel-kirienko]

┬─┬ノ(º_ºノ)

Suggested change

	using PayloadFragment = libcyphal::transport::PayloadFragment;
	using libcyphal::transport::PayloadFragment;

[pavel-kirienko]

Would be cool if we had something like IInvokable<...> defined in CETL and implemented by cetl::function, to enhance composability. The user could then either use a custom implementation for this interface, or use a cetl function.

[serges147]

Internally at CETL we already have something similar, see

namespace detail
{
template <typename Result, typename... Args>
class function_handler : public rtti_helper<function_handler_typeid_t>
{
public:
    virtual Result operator()(Args...) = 0;

};  // function_handler
...

but it has its issues which sooner or later we might need try to resolve. I'm talking about Args... args arguments which currently could be only copied but not moved (but I can't make it forward-able Args&&... args) - not sure yet how to address it, so it's a limitation currently).

[serges147]

Such assert is wrong - "swap" usually implemented by using move constructor and then 2 move assignments, like:

      _Tp __tmp = _GLIBCXX_MOVE(__a);
      __a = _GLIBCXX_MOVE(__b);
      __b = _GLIBCXX_MOVE(__tmp);

I found and fixed 3 such places in total.

[thirtytwobits]

this isn't actually an "observer" is it? Typically I reserve that term for things that are called back asynchronously and over time. This is more of a visitor pattern. It's basically foreach_fragment from what I can tell. If so; if this is similar to foreach, we should just use an std::function<void(PayloadFragment)> argument to make this consistent and easier to use.

[pavel-kirienko]

Surely you mean cetl::function?

[serges147]

@thirtytwobits Scott, I'll rename it to forEachFragment, but about making it cetl::function I have concerns. Surely, it was my initial thought to make it as cetl::function, but then I had second thoughts:

• cetl::function is not very cheap. my performance measurements showed that it's relatively cheap to call it, but creation is not so:
  • unbounded_variant behind the scene needs to be created (with several type-erasing "handler" function pointers being initialized)
  • cetl::rtti again behind the scene will be used to obtain internal callable interface
• normally, ScatteredBuffer will hold just couple of fragments; if it would be a lot of fragments then mentioned above creation overhead probably won't be noticeable, but in case of just a few fragments (1 for CAN, and N = data_size / UDP_MTU which is also like 1 or 2, depending on data_size of cause) it feels like a waste
• having IFragmentsObserver (or if you would like IFragmentVisitor) makes cost is just one virtual call to already existing visitor object.

What also concerns me in general is that in a lot of places of libcyphal we go extra mile for trying to achieve "zero-copy" of transferred data, or limit dynamic allocation, BUT at the same time we do a lot of copying and moving of various implicit unbounded_variants here and there, and IMO just move pressure from heap to stack. I know, heap fragmentation and etc., but still food for thoughts in my opinion.

So, what do you think about still having the interface after all?

[thirtytwobits]

Okay. let's so some renaming and you can keep the interface.
As for zero-copy in libcyphal; the use case we care most about is the ability to keep data in specific memory rather than requiring it to be moved into general program ram for libcyphal to use it. For example, when receiving frames that are hardware-managed buffers we want to keep the memory in these buffers until/unless the program specifically uses it to avoid the cost of moving for messages that are discarded (this allows low-cost, application-level message rate decimation). Another example is keeping all message data in a section of memory with a different cache policy than the program RAM. etc. As the project evolves we'll probably see snooping optimizations enabled which allows applications to filter messages based on an outer-data type's values without requiring full deserialization. All of this requires an architecture that ensures message memory is treated differently then general program RAM, heap, stack, or otherwise.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud