Re: Updated dataflow semantics for RTT

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-1972675851-1443526664=:3031
Content-Type: TEXT/PLAIN; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable

On Mon, 28 Sep 2015, Johannes Meyer wrote:

> Hi,
> thanks for all your input and ideas and I agree with Herman that the "o=
ne-framework-for-everything" approach of RTT
> is probably not how you would start a new project nowadays.


Some inputs from a discussion on the ROS mailinglist which is very much
related to this RTT-centric thread:
  <http://lists.ros.org/pipermail/ros-users/2015-September/069663.html>
  <http://lists.ros.org/pipermail/ros-users/2015-September/069664.html>

These are the critical remarks of two experienced people towards the
"one size fits all" approach of communication in ROS2.0; at the same time=
,
they also provide concrete use cases and trade-offs that are relevant for
the envisaged update of comunication in RTT...

Herman

PS I am of course more than a bit biased: both messages reflect also my
personal view on this issue of "communication middleware", and are not ju=
st
random samples from that ROS thread.

PS2 Geoff Biggs is another RTT-knowledgeable person who _did_ intervene i=
n
that ROS thread, as well as in this RTT thread. Are there others on this
list who, like Geoff and myself, are following both threads? Because,
together, we might "mine" both threads to extract very interesting inputs
from potential users, with widely varying expectations and contexts...

> But I would like to support Stephen's call to focus this
> discussion on the technical aspects of how to improve the existing impl=
ementation in as small as possible
> ("incremental")=C2=A0steps that add support for some new use cases that=
 are not covered yet (but have been partially
> covered by v1).

> Maintaining backwards compatibility is definitely the way to go and we =
can conclude that the new default semantics
> (especially per-connection vs. per-input buffers) have to be the same a=
s in previous v2 releases. I hope you find it
> acceptable that a new version would certainly be not ABI-compatible and=
 breaks some internal API calls that are
> typically not used directly by a user. This might break more advanced t=
hings like custom transport implementations.
> I fear that these kind of changes cannot be avoided completely, especia=
lly for the ConnFactory and ChannelElement
> classes.
>=20
> We should not limit the use cases of RTT on the core API level, like en=
forcing per-input storage objects for data
> connections and per-connection storage for buffer connections, but only=
 add new use cases. It is more a question of
> having good documentation and choosing reasonable defaults that cover a=
s many applications as possible, but at the
> end the application designer has to have the freedom to deploy the appl=
ication with whatever connection semantics it
> demands for.
>=20
> In the following I try to wrap up and comment on some of the concerns t=
hat came up during the discussion so far, and
> suggest compromise solutions which have been at least partially discuss=
ed with Peter:
>=20
> 1) There is definitely a need for per-input port buffers AND for per-co=
nnection buffers - INDEPENDENT of whether the
> data/buffer object is installed on the reader and writer side.
>=20
> Indeed, according to the original proposal the questions of at what sid=
e of a remote connection the data object will
> be installed and whether there will be a per-input or a per-connection =
buffer cannot specified separately. Peter and
> I could not imagine why you would want separate buffers if the data is =
already available locally anyways (in the
> push case) and we still think that at least for data connections the cu=
rrent per-connection implementation can be
> considered as broken. Even with readNewest() there is no guarantee that=
 you get the latest sample written. On the
> other hand, Sylvain correctly stated that there are no guarantees anywa=
y as soon as remote connections (or any
> non-realtime transport) are considered and the only true solution is pr=
oper timestamping and synchronization. But I
> think we agree that for most deployments with only local connections th=
e assumption that the latest sample is the
> last written sample holds and that timestamping is beyond the scope of =
RTT and can be left to user code or more
> advanced tools like the Rock stream aligner.
>=20
> Even with the proposed changes (let's call them v3 semantics as a worki=
ng title) the connection policy is still
> primarily describing the connection, even if *some* settings limit the =
set of allowed policies for other
> connections, making *some* other settings de-facto a per-port setting. =
This would be primarily true for input ports
> and not at all for output ports, unless it has a connection of the new =
shared type. But it is not an option to
> associate the connection policy *only* with a port because not *all* po=
ssible connections imply per-port buffers.
> Specifying a connection policy on a writer/reader pair base as it is no=
w and let certain invalid connection attempts
> fail is still the most general approach.
>=20
> Proposal: In order to not break existing applications, we introduce a n=
ew ConnPolicy field "read_policy" that
> toggles between per-input and per-connection buffers, instead of implyi=
ng that by the existing "pull" flag for
> remote connections. The read_policy would be an enum with two possible =
values for now, "SingleInputBuffer" and
> "PreferCurrentConnection". The reason why we propose an enum here is th=
at you could also think of other policies on
> how to select the next channel to read from in the per-connection case,=
 e.g. RoundRobin, PreferLastSignaled, etc.
> The current v2 default would correspond to PreferCurrentConnection, as =
the channel that was last read with new data
> is preferred by the input port and only then it polls all other connect=
ions in the order they have been made.
> We=C2=A0have not thought this idea to the end yet and the names of the =
constants are still under discussion. We do not
> want to call the input buffers "shared", because the read policy is ind=
ependent of the new concept of a "shared
> connection" where multiple input ports read from the same buffer. Or so=
meone can come up with a better name for this
> type of connection...
>=20
>=20
> 2) Should the new per-input port buffers become the new default or not =
(SingleInputBuffer read policy)?
>=20
> With that respect the conclusion is that backwards compatibility is mor=
e important than the new "features" and the
> default parameters for the current ConnPolicy class should not be touch=
ed. However, there are different ways to
> tackle this:
>=20
> 2.1 Make it a compile-time option.
>=20
> 2.2 Only add *new* fields to the ConnPolicy class without changing the =
default parameter values in the constructor
> and static methods, and a new class ConnPolicy2 or ConnPolicy3 that inh=
erits from ConnPolicy and might have other
> default parameters or another C++ API (I am thinking of a named-paramet=
er based API) and can also be used for
> scripting. Internally, the connection factory would work with const ref=
erences to the base class only.
>=20
> 2.3 We add a DefaultConnPolicy attribute to the GlobalsRepository. This=
 application-wide (per process) policy
> instance will be copied to newly constructed ConnPolicy instances and t=
he constructors and static methods only
> overwrite the respective parameters.
>=20
> It might be obsoleted by the above proposal, but in the meantime I adde=
d a compile-time option
> RTT_V2_COMPATIBILITY_MODE to my working branch (see=C2=A0ef99f2b),=C2=A0=
that, if set, reverts to the current v2 per-connection
> model, but lets RTT print meaningful warning messages in the case a new=
 connection would be invalid with v3
> semantics because of conflicts with other connections to the same port.=
 Such a flag could smooth the transition
> phase and help application designers to find the minimal set of connect=
ions that require connection policy changes.
>=20
> There is one small exception from the backwards-compatibility rule, tha=
t I think is worth discussing: =C2=A0The static
> method
>=20
> static ConnPolicy data(int lock_policy =3D LOCK_FREE, bool init_connect=
ion =3D true, bool pull =3D false);
>=20
> initializes the init_connection field to true by default, which was kin=
d of surprising me, because it means that
> every new connection to an output port with this policy will overwrite =
a sample previously written by another port.
> Is this what we really want as a default? It is also not consistent wit=
h the constructor called as
>=20
> ConnPolicy(DATA)
>=20
> which would initialize the init_connection flag to false. The static me=
thod data(...) is only used in the
> OutputPortInterface::createDataConnection(...) method from within RTT a=
nd OCL itself.
>=20
> By the way, the init_connection flag is a good example where the API of=
fers some degree of freedom to the
> application builder that semantically does not make sense at all for ce=
rtain combinations, like in combination with
> a buffer connection. It is still not forbidden to set the flag, but it =
might be worth a warning log message if a
> connection is created like this.
>=20
>=20
> 3) How to add the "shared" concept within the current API, or to use a =
"new" API?
>=20
> This point was brought up by Janosch, who asked for "a more explicit in=
terface for shared connections", with the
> main argument that it is fundamentally different=C2=A0from a=C2=A0norma=
l connection and a "connect port A to B with policy C"
> kind of API does not fit well to that concept. That's true. We consider=
ed to use the createStream(...) API first,
> with a name_id string that identifies the shared buffer to connect to. =
This would actually fit better to the shared
> connection model, but it still enforces the same buffer type policies f=
or all connections. We withdraw that idea
> because the existing API is more general and disallowing certain policy=
 combinations is required anyway for private
> push connections with a single input buffer. At least in this case it c=
annot break existing applications.=C2=A0I do not
> think that there is a need for a completely new API only for shared con=
nections.
>=20
> My current implementation in the updated-dataflow-semantics branch=C2=A0=
is as follows, and new/other ideas are always
> welcome:
>=20
> - Shared connection objects (the shared data object or buffered) are ad=
ded to a process-wide registry. Every shared
> connection is identified by a unique string, either set explictly in th=
e name_id field of the ConnPolicy or assigned
> automatically, in which case the name of the connection is returned in =
exactly this field (name_id is and always was
> declared as mutable).
> - For new connections with the name_id set, the shared connection is lo=
oked up in the registry, or created as new.
> As a consequence, if the same ConnPolicy instance is used to make a sec=
ond connection, even if name_id was not set
> explicitly before, you will always connect to the same connection objec=
t.
> - For new connections with no name_id set, the port objects provide an =
API to return a pointer to an existing shared
> connection if they are already connected to an existing one, and only t=
he non-connected port will be added. So ports
> can be connected in any order to each other, as long as there is always=
 one of the two ports already connected.
> Otherwise the name_id has to be set or a new connection instance would =
be created. Perhaps this one could be dropped
> because it it too implicit and does not work very well for remote conne=
ctions...
> - If an existing connection was found, the buffer policies (type, size =
and locking policy) have to match.
> - All ports will only store a single pointer to the shared connection i=
nstance in their connection list (in
> internal::ConnectionManager) and cannot know directly which other ports=
 are connected.
> - For remote input ports with a shared connection, only a proxy is crea=
ted locally and can be looked up in the
> shared connection repository. Other ports will only connect to either t=
he =C2=A0proxy or the real shared connection
> instance, but never build a new remote channel once the proxy has been =
created for the first remote connection
> (works in both directions).
> - All connect and disconnect calls to shared connections are logged wit=
h log level Debug to the RTT logger, so at
> least there is a manual way to check whether the resulting connections =
have been made as expected. Graphical tools
> like the rtt_dot_service=C2=A0could be patched later to visualize share=
d connections correctly.
>=20
>=20
> 4) Whether to encode the result of a write() in existing enums or not?
>=20
> Janosch asked for introducing a separate enum for the return value of w=
rite(...) calls, in order to not break or
> trigger compiler warnings for existing code that already uses the FlowS=
tatus enum in a switch statement. Another
> minor issue with a single enum is the default value of FlowStatus attri=
butes, which can be only meaningful for one
> use case.
>=20
> That's also fine for me and I will update the document and implementati=
on accordingly (add a new enum WriteStatus).
> Note that the evaluation in boolean context is counter-intuitive in thi=
s case, other than for FlowStatus, because
> the value WriteSuccess (=3D0) value evaluates to false and all error ca=
ses (WriteFailure and NotConnected) evaluate to
> true.
>=20
>=20
> I hope I did not forget anything. This mail already became much longer =
than expected. I am sorry.
>=20
> I also did some performance measurements of read and write calls last w=
eek in terms of absolute time, CPU time,
> number of assignments and so on of the current, the new and even the v1=
 data flow implementations in different
> scenarios. I plan to come up with some results during this week. The te=
sts revealed some bugs and performance
> caveats, and some of them are already present since the early days of R=
TT v2, e.g. that data samples might be lost
> (never read as NewData) because the read flag is not lock-protected or =
stored within the lock-free data structure. I
> am preparing a pull request to the current master branch, but it will a=
lso break the API of class
> base::DataObjectInterface.
>=20
> Best regards,
> Johannes
>=20
>=20
>=20
> On Mon, Sep 28, 2015 at 3:36 PM, S Roderick <[email protected]> wrote:
>=20
> Johannes' proposed changes are focussed on
> a) push vs pull, or specifying whether the data/buffer object is on the=
 reader or writer side. This is very
> useful for system designers dealing with significant time delay.=C2=A0
> b) private vs shared, whether a connection is per input/output port pai=
r or whether the connection is shared
> between multiple input/output ports.
> c) mandatory, primarily intended to support knowing whether writes to a=
 buffer succeeded (so that traditional
> producer/consumer relationships have some knowledge about the buffer st=
ate)
>=20
> It appears to me that most people are worried about changing the defaul=
ts, more than they're worried about the
> actual proposed changes. Actual technical concerns with the proposed ch=
anges appear to be
>=20
> 1) whether to encode the result of a write() in existing enums or not
> 2) how to add the "shared" concept within the current API, or to use a =
"new" API
> 3) changing the RTT defaults, which then affect an application's semant=
ics.
>=20
> Did I miss any technical issues?
>=20
>=20
> My huge personal caveat to all of this is that I do *not* think we shou=
ld change the current system semantics
> in introducing this change. Too many Orocos changes have broken backwar=
ds compatibility and forced system
> designers and implementors to change their system in response. I don't =
think we should force that on users
> without an incredibly good reason. Otherwise it breeds frustration in t=
he community.
>=20
> If there is some way to introduce the proposed changes to the system, b=
ut to not change the behavior of
> current systems, then I think that is a win for all. There does seem to=
 be some general agreement that there
> are valid use cases that the current port implementation handles poorly=
, and that this proposed approach does
> fix some of those. The proposed approach also improves the situation fo=
r systems that don't want or need to
> use the flow status approach. Both are equally valid approaches - IMHO =
it would be beneficial to the community
> as a whole if RTT supported both approaches.
>=20
> Cheers
> Stephen
>=20
>=20
> --
> Orocos-Dev mailing list
> [email protected]
> http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev
>=20
>=20
>=20
>
--8323329-1972675851-1443526664=:3031
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

-- 
Orocos-Dev mailing list
[email protected]
http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev

--8323329-1972675851-1443526664=:3031--