CORBA efficiency patches #123

meyerj · 2015-12-15T18:12:08Z

This PR is the result of some work we did last year to optimize the CORBA inspection and transport, especially for systems where the network is affected by transport delay and packet loss. Actually I already wanted to open a PR to merge into master since a long time, but now #122 reminded me that I actually never did that. This branch has already been used in some bigger deployments and I would consider it as ready to be merged.

The existence of this patch has already been mentioned in a side note during the update dataflow semantics discussion on the mailing list: http://www.orocos.org/forum/rtt/rtt-dev/updated-dataflow-semantics-rtt#comment-36662

Major updates

Most changes in the CORBA transport affect the way a TaskContextProxy synchronizes its objects (ports, operations, properties, services etc.) with the remote server. The number of CORBA calls going over the network has been greatly reduced. In fact, the proxy requests one big TaskContextDescription object in a single call now instead of enumerating all task objects subsequently.

Furthermore ~~write() and~~ (optional, reverted in #151) remoteSignal() calls for port connections via a CORBA transport have been converted to oneway calls, which means that the writer (the CORBA dispatcher of the writing TaskContext) is not blocking anymore (see also #122). The signalling can be disabled completely if appropriate (see section Optional patches).

Caveats

The IDL is not compatible to previous RTT releases, so both sides of a connection have to updated synchronously.

Optional patches

The following patches are only enabled on demand (at compile time of RTT) as they can change the behavior of the application, beyond pure optimization. They can be enabled by setting the corresponding Cmake variables to ON. Default state is OFF.

PLUGINS_CORBA_PORTS_DISABLE_SIGNAL

When a port is connected to a remote port with the pull flag of the ConnPolicy set to true, the writing side normally signals the reader to notify him about new data. If PLUGINS_CORBA_PORTS_DISABLE_SIGNAL is enabled, the remoteSignal() call is skipped. This is only for saving bandwidth if data is written periodically anyway. The subsequent read() call to fetch the data blocks the reader, so pulling connections are not well suited for robot to GUI connections anyway unless the GUI reads only on demand or there is a separate thread that periodically reads all input ports.

Note that adding input ports which are only connected through a remote connection to a component as an event port will not trigger the component if remote signalling is disabled.

PLUGINS_CORBA_NO_CHECK_OPERATIONS

Normally the remote side checks the validity of the signature whenever it produces a handle for a remote operation call (or send), so for every OperationCaller connection in C++ or every call to a remote operation in scripting. The PLUGINS_CORBA_NO_CHECK_OPERATIONS cmake flag will disable this check, which means that wrong signatures cannot be detected while the OperationCaller is connected (usually in the configureHook or startHook) or when a script is loaded. Calling or sending an operation with a wrong signature will results in an exception during runtime instead.
Omitting the checkOperation() calls greatly reduces the setup time for the GUI (approx. by factor 2).

PLUGINS_CORBA_SEND_ONEWAY_OPERATION

This cmake switch enables oneway sending of operations which have a collect arity of zero (return void and have no reference arguments). The advantage is that the send() call does not block until the CSendHandle is returned. On the other side, without the handle it is impossible to check whether the operation has finished or if an exception occured. Exceptions or cases where an operation has been send with the wrong signature (if PLUGINS_CORBA_NO_CHECK_OPERATIONS is also active) at least prints an error log message at the server side.
Calling operations is not affected by this patch and always blocks until the operation finishes and the result is returned by the remote side.

PLUGINS_CORBA_PORTS_WRITE_ONEWAY

Although port writes could be communicated through oneway method calls on the CORBA layer for efficiency, the original patch (08b77c4) which applied this change unconditionally broke the guaranty that input ports read the samples in the same order as they have been written to the output port. Due to multi-threading in the CORBA implementation (depending on its implementation and its configuration) this guaranty did not hold anymore, which caused issues (discussion starting in #123 (comment)) and occasional unit test failures.

Therefore this part has been reverted in #151 and replaced by an optional build-time configuration variable PLUGINS_CORBA_PORTS_WRITE_ONEWAY.

Signed-off-by: Johannes Meyer <[email protected]>

…formation in one remote call per TaskContext Signed-off-by: Johannes Meyer <[email protected]>

Signed-off-by: Johannes Meyer <[email protected]>

…on description Signed-off-by: Johannes Meyer <[email protected]>

Signed-off-by: Johannes Meyer <[email protected]>

…S to enable or disable oneway operations If PLUGINS_CORBA_SEND_ONEWAY_OPERATIONS is set to ON, operations that have a collect arity of zero (void return value and no reference arguments) are sent as oneway calls. The fake SendHandle returned to the caller will always evaluate to SendSuccess, as if the operation would have been completed successfully. Note that exceptions in the operation implementation are not forwarded to the caller and print a log message at the server's side only. Signed-off-by: Johannes Meyer <[email protected]>

…to disable remote signalling If PLUGINS_CORBA_PORTS_DISABLE_SIGNAL is set to ON, writing into ports that are connected to a remote port via the CORBA transport will not signal the remote side or trigger the owning TaskContext. The remote side has to poll the port periodically. Signalling is only active if pull is set to true in the ConnPolicy. Otherwise every write() is pushed to the remote side and PLUGINS_CORBA_PORTS_DISABLE_SIGNAL has no effect. Signed-off-by: Johannes Meyer <[email protected]>

…o disable operation signature checking If PLUGINS_CORBA_NO_CHECK_OPERATIONS is set to ON, producing call and send handles for remote operations will not check the argument types. Instead, calling or sending operations with an incompatible signature will trigger an exception at runtime. Signed-off-by: Johannes Meyer <[email protected]>

meyerj · 2015-12-15T19:05:19Z

rebased on master

…entation Signed-off-by: Johannes Meyer <[email protected]>

orocos-toolchain/rtt#123

planthaber · 2015-12-16T08:13:00Z

rtt/transports/corba/OperationInterface.idl

@@ -125,17 +137,18 @@ module RTT
     */
    interface COperationInterface
    {
-	typedef sequence<string> COperationList;
+      typedef sequence<string> COperationList;


Had to adapt rock's ruby bindings to this PR for my test in #122

Had to switch to COperationDescriptions from COperationList and found that the COperationList typedef here is still defined but not used anymore.

Indeed, COperationList should be removed.

doudou · 2015-12-18T11:09:39Z

Since it's incompatible for e.g. the Rock bindings, would it be possible to have a way to test for the change at the preprocessor level ? That would allow to be compatible with both the new and old interface.

meyerj · 2015-12-18T14:33:26Z

I started to create toolchain-2.9 branches which have the corba-efficiency-patches (#123), the updated-dataflow-semantics (#114) and also the new RTT_VERSION_GTE preprocessor macro (#115) already integrated. With this branch you could compile tools-orocosrb conditionally:

#if RTT_VERSION_GTE(2,9,0)
// ...
#endif

There might be more small API changes that require adaptations in the Ruby bindings for rtt 2.9.
Or do you had something else in mind with "a way to test for the change at the preprocessor level"?

I am currently working on the Toolchain 2.9 release which has most of the open PRs with API breaking changes merged in. Peter already wrote down some release notes (orocos-toolchain/orocos_toolchain#12). I wanted to ask the orocos-dev mailing list for comments during the next days, especially what branch/tag of typelib and orogen should be considered as stable. For now I am using the rock-15.05 tag present in most of the repos.

…face Signed-off-by: Johannes Meyer <[email protected]>

doudou · 2015-12-21T12:40:31Z

The problem with orogen is that you still have a pretty heavily forked version of it. The major difference it will make for you between rock-15.05 and master is that master is "ready" for clang (not meaning that it works, but at least most of the ground work is there)

meyerj · 2015-12-21T21:31:45Z

From #122 (comment):

It should be noted that with the merged branches corba-efficiency-patches (#123) and updated-dataflow-semantics-rebased (#114) in toolchain-2.9 writes are only send as one-way calls if the connection is not mandatory (see new flag mandatory in ConnPolicy.hpp). The differentiation happens in RemoteChannelElement.hpp:351.

After I wrote this I noticed that because all writes through CORBA are buffered and forwarded by the CorbaDispatcher thread, the WriteStatus return value as returned by the RemoteChannelElement is invisible to the caller anyway. In that case we could also stick to one-way writes in all cases. I updated the merge commit in 3643898.

On the other hand it must be clearly documented that failures during transport (e.g. packet loss) or at the remote side (e.g. full input buffer) are not reflected in the WriteStatus returned to the original writer.

meyerj · 2016-02-12T18:35:00Z

Just found another caveat when writes are oneway: There is no guaranty on the order of arrival of write operations anymore:

Task A: 1.588 [ Debug  ][Thread] RemoteChannelElement(0x111f8d0): write(6.33) -> CORBA
Task A: 1.588 [ Debug  ][Thread] RemoteChannelElement(0x111f8d0): write(6.33) finished.
Task A: 1.588 [ Debug  ][Thread] RemoteChannelElement(0x111f8d0): write(3.33) -> CORBA
Task A: 1.588 [ Debug  ][Thread] RemoteChannelElement(0x111f8d0): write(3.33) finished.
Task B: 380.228 [ Debug  ][SendHandleC] RemoteChannelElement(0x7fb89c1b19a0): CORBA -> write(3.33)
Task B: 380.228 [ Debug  ][SendHandleC] RemoteChannelElement(0x7fb89c1b19a0): CORBA -> write(6.33)
Task B: 380.228 [ Debug  ][SendHandleC] Received 3.33
Task B: 380.228 [ Debug  ][SendHandleC] Received 6.33

doudou · 2016-02-12T18:45:23Z

Indeed: http://www.omniorb-support.com/pipermail/omniorb-list/2005-April/026538.html

meyerj · 2016-02-12T18:45:47Z

Since it's incompatible for e.g. the Rock bindings, would it be possible to have a way to test for the change at the preprocessor level ? That would allow to be compatible with both the new and old interface.

@doudou What exactly would be incompatible? Can you point me to some code? At the Port or TaskContext(Proxy) API level you should not see a difference. Of course the CORBA IDL and the RemoteChannelElement API changed.

Anyway, you can check the version of RTT with a macro introduced in #115, so that if this PR would be applied to version 2.9 only, you can write code like

#include <rtt/rtt-config.h>

#ifdef RTT_VERSION_GTE
#if RTT_VERSION_GTE(2,9,0)
    // with corba-efficiency-patches
#endif
#else
    // without corba-efficiency-patches
#endif

There are no preprocessor macros that allow conditional compilation directly based on the availability of certain RTT features.

Signed-off-by: Luca Magnabosco <[email protected]>

corba: avoid some corba calls improving getArgumentList()

… the remote side has no CORBA transport for that type Like bfd3184 for buildChannelOutput(). The same problem can occur in buildChannelInput(). type_info->getProtocol(ORO_CORBA_PROTOCOL_ID) always returns a valid CorbaTypeTransporter, which might be of type CorbaFallBackProtocol. So the check a few lines above is not effective and we can only know that there is no specific CORBA transport for that type after createChannelElement_i() returned a null pointer, which without the added check would have caused a segfault. Signed-off-by: Johannes Meyer <[email protected]>

…rFactory::getArgumentType() This commit fixes a regression from 3b1e482. COperation mdescription contains fully-qualified argument types as returned by OperationInterfacePart::getArgumentList() and DataSourceTypeInfo<T>::getType(), while COperationInterface::getArgumentType() would return the unqualified type name as DataSourceTypeInfo<T>::getType(). Signed-off-by: Johannes Meyer <[email protected]>

…write call This is especially required if the other end of the connection disappeared, in order to properly cleanup and to not spam the log with transient CORBA errors. If required, it is up to the user application to reestablish the connection automatically in case the connection breaks. Instead, we check the return value of updateAny/updateFromAny and return a WriteFailure in case conversion fails. This is most likely a temporary error and not due to a broken CORBA connection. Signed-off-by: Johannes Meyer <[email protected]>

This reverts commit 3643898, which removed the writeOneway() method from the CChannelElement interface. With oneway writes we cannot guarantee that samples are never reordered even on a single connection due to potential multi-threading within the CORBA implementation. This patch introduces a new cmake flag PLUGINS_CORBA_PORTS_WRITE_ONEWAY, which enables oneway writes and has to be set explicitly when RTT is configured. Otherwise the old behavior of using standard two-way calls is restored. Note that it is not possible to configure this during run-time or even per-connection, which would require an extension in the ConnPolicy struct. The CORBA transport option flags have been added to the generated rtt-corba-config.h header file. This allows to use them in header files required to generate transport plugins in dependent packages (see also 2f34384) and removes the need to pass them as individual constructor arguments to new RemoteChannelElement instances. Instead, every RemoteChannel instance has a full copy of the ConnPolicy struct now, which allows to adapt its run-time behavior in a more flexible way without the need to change the signature of the constructor or factory methods in the future. Signed-off-by: Johannes Meyer <[email protected]>

Tested with tao 2.3.1: - fixed Service.idl, line 35: Error: anonymous types are deprecated by OMG spec - fixed ServiceRequester.idl, line 31: Error: anonymous types are deprecated by OMG spec - fixed different exception specifier in implementation of CDataFlowInterface_i::createSharedConnection() - some whitespace indentation updates - removed server inline skeleton files from the list of files to be generated and installed in the ORO_ADD_CORBA_SERVERS() cmake macro for TAO. tao_idl does not generate these files and hence cmake complains when it tries to install them. Signed-off-by: Johannes Meyer <[email protected]>

corba: revert oneway writes and fixed connection cleanup and TAO builds

meyerj · 2017-10-17T00:09:55Z

Re-merged into toolchain-2.9 in 171ed58.

Signed-off-by: Johannes Meyer <[email protected]>

…e synchronization of component interfaces

Signed-off-by: Luca Magnabosco <[email protected]>

…-pick CORBA efficiency patches Added two patches: ea9de93 transports::corba fix crash retrieving property name through corba 0175877 corba: improve log output in case of unsufficient type info during the synchronization of component interfaces

…fficiency-patches

meyerj · 2019-04-11T16:03:22Z

@planthaber @doudou Do you agree with merging this old patch along with a version bump to at least 2.9 into master and to follow-up on the remaining issues about partitioning the remote connections in #227 and #248?

This pull request is mainly about the more efficient communication of the TaskContext description and the oneway writes have been reverted due to reordering issues (#151) (unless RTT is compiled with PLUGINS_CORBA_PORTS_WRITE_ONEWAY).

planthaber · 2019-04-11T16:45:34Z

I don't see an issue with the merge

planthaber · 2019-04-11T16:45:46Z

I don't see an issue with the merge

planthaber · 2019-04-11T16:45:55Z

I don't see an issue with the merge

doudou · 2019-04-15T13:32:02Z

Sounds good to me too.

meyerj added the in progress label Dec 15, 2015

meyerj self-assigned this Dec 15, 2015

meyerj added this to the 2.9 milestone Dec 15, 2015

meyerj added 9 commits December 15, 2015 19:54

corba: use oneway for remoteSignal() and write() calls

08b77c4

Signed-off-by: Johannes Meyer <[email protected]>

corba: make synchronization much faster by collecting all required in…

22a6b16

…formation in one remote call per TaskContext Signed-off-by: Johannes Meyer <[email protected]>

corba: added oneway implementation of sending operations

62b63a4

Signed-off-by: Johannes Meyer <[email protected]>

corba: minor updates to save some more corba calls

a8b8b37

Signed-off-by: Johannes Meyer <[email protected]>

corba: added full list of arguments and collect types to the COperati…

a3e491e

…on description Signed-off-by: Johannes Meyer <[email protected]>

corba: fixed exception handling when sending oneway operations

f9cef0c

Signed-off-by: Johannes Meyer <[email protected]>

meyerj force-pushed the corba-efficiency-patches branch from 23331ec to b3e9db2 Compare December 15, 2015 19:05

meyerj force-pushed the corba-efficiency-patches branch from b3e9db2 to 0c200be Compare December 15, 2015 19:10

meyerj mentioned this pull request Dec 15, 2015

enable corba oneway communication for write() #122

Closed

corba: moved CORBA_PORTS_DISABLE_SIGNAL checks from headers to implem…

2f34384

…entation Signed-off-by: Johannes Meyer <[email protected]>

meyerj force-pushed the corba-efficiency-patches branch from 0c200be to 2f34384 Compare December 15, 2015 19:31

planthaber added a commit to planthaber/tools-orocosrb that referenced this pull request Dec 16, 2015

adaptations for PR 123 on rtt

7a39846

orocos-toolchain/rtt#123

planthaber reviewed Dec 16, 2015
View reviewed changes

planthaber mentioned this pull request Dec 16, 2015

adaptations for PR 123 on rtt rock-core/tools-orocosrb#91

Merged

meyerj mentioned this pull request Dec 18, 2015

New Toolchain releasenotes orocos-toolchain/orocos_toolchain#12

Merged

corba: removed deprecated typedef COperationList from COperationInter…

2117f50

…face Signed-off-by: Johannes Meyer <[email protected]>

Luca Magnabosco and others added 9 commits August 23, 2017 10:21

corba: avoid some corba calls improving getArgumentList()

2057af7

Signed-off-by: Luca Magnabosco <[email protected]>

Merge pull request #8 from MagnaboscoL/corba-minor-efficiency-patch

f1ba54d

corba: avoid some corba calls improving getArgumentList()

Backport of pull request orocos-toolchain#151

66c9e03

corba: revert oneway writes and fixed connection cleanup and TAO builds

Merge branch 'master' into corba-efficiency-patches

64bc821

tests: unified ASSERT_PORT_SIGNALLING and wait_for_* macros

01d899a

Signed-off-by: Johannes Meyer <[email protected]>

meyerj force-pushed the corba-efficiency-patches branch from 313d376 to 44e5ce1 Compare October 22, 2017 00:22

tests: disable some checks depending on plugin options

83b9770

Signed-off-by: Johannes Meyer <[email protected]>

meyerj force-pushed the corba-efficiency-patches branch from 44e5ce1 to 83b9770 Compare October 22, 2017 00:44

corba: improve log output in case of unsufficient type info during th…

eee2edb

…e synchronization of component interfaces

meyerj mentioned this pull request Oct 27, 2017

Fix/typeinfo retrival and other minor meyerj/rtt#5

Closed

transports::corba fix crash retrieving property name through corba

a25c063

Signed-off-by: Luca Magnabosco <[email protected]>

meyerj mentioned this pull request Oct 27, 2017

transports::corba fix crash retrieving property name through corba meyerj/rtt#7

Closed

Merge branch 'master' of github.com:orocos-toolchain/rtt into corba-e…

a9a92fb

…fficiency-patches

meyerj mentioned this pull request Nov 13, 2017

types: fix type lookup for qualified types #223

Closed

meyerj mentioned this pull request Dec 5, 2017

Lost messages when a port hast multiple connections (one remote connection) #248

Open

Merge branch 'master' into corba-efficiency-patches

90c863c

meyerj merged commit f0bd90c into orocos-toolchain:master May 8, 2019

meyerj deleted the corba-efficiency-patches branch May 8, 2019 21:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CORBA efficiency patches #123

CORBA efficiency patches #123

meyerj commented Dec 15, 2015 •

edited

Loading

meyerj commented Dec 15, 2015

planthaber Dec 16, 2015

meyerj Dec 18, 2015

doudou commented Dec 18, 2015

meyerj commented Dec 18, 2015

doudou commented Dec 21, 2015

meyerj commented Dec 21, 2015

meyerj commented Feb 12, 2016

doudou commented Feb 12, 2016

meyerj commented Feb 12, 2016

meyerj commented Oct 17, 2017

meyerj commented Apr 11, 2019

planthaber commented Apr 11, 2019

planthaber commented Apr 11, 2019

planthaber commented Apr 11, 2019

doudou commented Apr 15, 2019

CORBA efficiency patches #123

CORBA efficiency patches #123

Conversation

meyerj commented Dec 15, 2015 • edited Loading

Major updates

Caveats

Optional patches

PLUGINS_CORBA_PORTS_DISABLE_SIGNAL

PLUGINS_CORBA_NO_CHECK_OPERATIONS

PLUGINS_CORBA_SEND_ONEWAY_OPERATION

PLUGINS_CORBA_PORTS_WRITE_ONEWAY

meyerj commented Dec 15, 2015

planthaber Dec 16, 2015

Choose a reason for hiding this comment

meyerj Dec 18, 2015

Choose a reason for hiding this comment

doudou commented Dec 18, 2015

meyerj commented Dec 18, 2015

doudou commented Dec 21, 2015

meyerj commented Dec 21, 2015

meyerj commented Feb 12, 2016

doudou commented Feb 12, 2016

meyerj commented Feb 12, 2016

meyerj commented Oct 17, 2017

meyerj commented Apr 11, 2019

planthaber commented Apr 11, 2019

planthaber commented Apr 11, 2019

planthaber commented Apr 11, 2019

doudou commented Apr 15, 2019

meyerj commented Dec 15, 2015 •

edited

Loading