Giter VIP home page Giter VIP logo

Comments (17)

jhelovuo avatar jhelovuo commented on August 22, 2024 1

i've been looking at this a little further (and let me preface this by first saying i'm emphatically not an expert in low-level network programming!). I think this crate has some very tight coupling between the various concerns within DDS, and would benefit immensely by greater separation of the

They could be separated to clarify the design, but separate crates would AFAIK bring no reuse advantage. This is because the networking layer and RTPS protocol are unique to DDS, so there would be no use elsewhere. Network programming is typically not deep magic. It is just

  1. packet generation (serialization) and parsing (deserialization), and
  2. behaviour, or how to respond to each incoming message or API call. This latter one may contain devils in the details.

The division of source code into subdirectories is inherited from the rtps-rs project, and it seems to be a verbatim copy of the division in DDS/RTPS specifications, which is not necessarily optimal for the implementation.

In order to do DDS/RTPS, one would need the following layers:

  • Application-facing API. This is what the DDS specification is about. Currenly, these are the public APIs of DomainParticiapnt, Publisher, Subscriber, DataReader, DataWriter, Topic, and QosPolicies. Also the Error type is in this category.

  • Data caching and communication between API layer (e.g. DataReader) and the network behaviour layer (e.g. Reader). These are currently DDSCache (global), DataSampleCache (one per DataReader/DataWriter), various channels between these. The global cache contains serialized objects (CacheCahnges) or notifications of their disposal. The local caches contain deserialized data.

  • Discovery. This is a background process that uses DDS itself with predefined topics, readers, and writers to communicate to discover the other participants, their readers, writers, and topics. Discovery is a bit tricky, because it both uses DDS and is part of the service provided by DDS. Discovery consists of behaviour and DiscoveryDB of discovered data.

  • Payload serialization/deserialization. RTPS specifies CDR encoding, which we implement in cdr_serializer and cdr_deserializer using Serde. Spec also says that the application must be able to replace CDR by something application-specific if needed.

  • Fragmentation/reassembly layer. This is needed for serialized objects that do not fit into a single UDP packet. Currently only the reassembly (datafrag receive) is implemented. We still need receive processing for nackfrag, heartbeatfrag, and transmit processing for all of these.

  • Protocol serialization/deserialization (or call it message parsing and generation). This consists of submessage objects (messages subdirectory) and these consist of submessage elements (structure subdirectory), such as GUID, Locator, Timestamp, SequenceNumber, etc. This is done using Speedy, although some of the Readable and Writable implementations are hand-crafted, because the Speedy API could not convey what was needed.

  • Multicast networking layer: UDPSender, UDPListener. This is very simple now. It does not do any clever packet buffering or combining.

Threadwise, there are three (or more) threads:

  • Background networking (RTPS processing) thread. This is in dp_event_loop. There is one such thread per DomainParticipant.
  • Background Discovery thread. Runs the Discovery protocols over DDS. One thread per DomainParticipant.
  • Application thread that calls us. One or more such threads.

These threads mainly communicate using mio::channels, but payload data is transferred through DDSCache, i.e. a shared data structure. Both background threads have their own mio::polls. The dp_event_loop polls network sockets, timers, and channels from other threads. Discovery polls DDS DataReaders, timers, and channels from others. Application thread is given pollable DataReader and DataWriter objects.

In order to make things async, some (all?) of this should be replaced by async tasks. I have been considering implementing an async adapter layer on top of the current code, but I am not yet sure if it is doable.

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024 1

Sounds like a lot of work, but a chance to learn a lot on the way.

Just check that you have the latest specs, i.e. DDS 1.4 and RTPS 2.5. If you googled them, it is very easy to find outdated versions. RTPS is designed so that 2.5 (and later 2.x) are backwards-compatible down to 2.0.

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024

The original design decision to use mio was made some years ago (2019?) when asyncs were not yet up to the task.

Are you suggesting that the application-facing API would be changed to use async, or the internal implementation of RustDDS, or both?

Using mio is a bit bothersome, because we need features from mio 0.6.x that were dropped in version 0.7 onwards.

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

Are you suggesting that the application-facing API would be changed to use async, or the internal implementation of RustDDS, or both?

both. unless there's a specific reason not to, such as targeting some bare-metal environment without the default allocator. Even then i think a lot of work has been done to use async in no-std environments (i'm by no means an expert on this)

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024

Yes, it would be an advantage to have an async application API, so RustDDS could be used by async applications directly.

But moving the implementation itself on top of async would force us to choose a runtime, Tokio, async-std, or some other. Or is there a way to do this in a manner that fits several runtimes? RustDDS requires network I/O, internal background threads (currently 2), internal inter-thread channels, and timers, at least.

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

Yes, it would be an advantage to have an async application API, so RustDDS could be used by async applications directly.

But moving the implementation itself on top of async would force us to choose a runtime, Tokio, async-std, or some other. Or is there a way to do this in a manner that fits several runtimes? RustDDS requires network I/O, internal background threads (currently 2), internal inter-thread channels, and timers, at least.

the short answer is "yes", it's possible to make it runtime agnostic.

the slightly longer answer is that it's trivially possible if the library doesn't need to spawn tasks (which is the case for most libraries) since you simply return the futures and let the consumer decide how to run them. If you do need to spawn tasks, you can expose a trait for doing so. that's the strategy that Hyper uses, for example - https://docs.rs/hyper/0.14.13/hyper/rt/trait.Executor.html

another option us to use an executor with a compat layer. For example smol can run libraries that use the tokio global executor - https://docs.rs/async-compat/0.2.1/async_compat/

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024

Does "spawn tasks" mean here approximately the same as "start background threads", except using the async mechanisms instead of OS threading?

I think that is precisely what we would need, because DDS must be able to run network communications and discovery in the background, even if the application is otherwise just blocking and waiting for data.

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

yes, spawning a task is the async equivalent of spawning a background thread

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

i've been looking at this a little further (and let me preface this by first saying i'm emphatically not an expert in low-level network programming!). I think this crate has some very tight coupling between the various concerns within DDS, and would benefit immensely by greater separation of the

  • message definitions
  • serialisation/deserialisation
  • networking

I kind of think these should be separate crates within the workspace.

For someone like me, who doesn't really know what they're looking at, or where the boundaries between concerns lie, it's very hard to see how to refactor this away from mio, and towards say, tokio and tower

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

speaking of, what's the relationship between this crate and rtps-rs?

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024

Technically, this crate is a fork of rtps-rs.

Practically, this crate is a working DDS/RTPS implementation, whereas rtps-rs is an work-in-progress RTPS implementation and no DDS layer.

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

@jhelovuo that's a brilliant explanation, that helps a lot.

I'm going to try and crawl before i try and run, so with that in mind i've gone back to basics and am familiarising myself with the spec. I'm also playing around with some ideas here -> https://github.com/danieleades/rtps, in order to better wrap my head around it, and understand the design choices in this repo. You might be interested to follow my progress. I'm primarily using the specification directly, with some reference to the existing implementation. I'm looking at the "structure" now, and then will move on to the other modules. I'm kind of reinventing the wheel, but it's helping me understand it.

My hope is that when i come to the 'discovery' module, that it will become apparent how to implement this using a higher-level asynchronous model. Or maybe it'll turn out that the lower-level mio approach was the right one all along. I'll find out when i get there!

Either way, if i do find any possible improvements i'd be happy to merge them back in, but they're undoubtedly going to be breaking changes. Perhaps a dev or release branch would be useful for exploring breaking changes?

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

I would love to start replacing mio with tokio, and removing the polling and manual event loops. I have to admit it's a little daunting.

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024

I have an alternative idea, although it needs some refinement:

We could keep the current dp_event_loop and discovery event loop, but provide an alternative, async API so that RustDDS could be used from an application written using async code.

Many of the necessary operations, such as construction of DomainParticiapant , Subscriber, DataReader, QosPolicies, etc. are non-blocking. They can still be usual synchronous code. (Analogously for publisher/datawriter side.)

There are some operations that need async equivalents of the current pollable versions, because they can block. Most important ones are read()and take()methods in DataReader, along with some others, such as wait_for_historical_data() and try_recv_status().

The blocking operations mostly block because they talk to the back end (dp_event_loop) using mio channels. A first step would be to get some channels that can be used to convey data between synchronous dp_event_loop and async tasks. Another thing to resolve would be how to write async versions of the necessary API functions without much code duplication.

from rustdds.

danieleades avatar danieleades commented on August 22, 2024

that solves the problem of providing an external asynchronous API, but it doesn't solve the problem of simplifying the internal implementation. I guess those are separate concerns (though I think an async/await-based implementation has the potential to solve both problems)

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024

Yes, they are separate concerns.

Changing the internal implementation to be async-based would be a major rewrite, and then RustDDS would be usable only under some async executor.

Currently, there is some internal complexity, but

  • it already exists and works, and
  • I do not see async code as a magic bullet that would make it vastly simpler.

If we manage to add an alternative async API, then usage without any async apparatus is also possible.

from rustdds.

jhelovuo avatar jhelovuo commented on August 22, 2024

Async is now supported, so closing.

from rustdds.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.