dart icon indicating copy to clipboard operation
dart copied to clipboard

Proposal: Add inline namespaces to DART and maintain ABI stability between minor releases

Open mxgrey opened this issue 6 years ago • 4 comments

This is semi-related to #917

Future versions of Gazebo intend to use a plugin-based architecture in order to make the Gazebo simulations more modular and customizable.

One danger that is often faced is that multiple plugin libraries might depend on different versions of the same dependencies. Trying to use these multiple plugins simultaneously in one application can easily result in ABI collisions, due to DART's current approach to ABI management.

An easy and effective way to avoid symbol collision is to use inline namespaces which contain the major version of the library. However, this is only effective for avoiding symbol collisions between different major versions.

If the DART ABI gets changed between minor versions (e.g. adding a new member variable to a class), then simultaneously using two plugins that were built against two different minor versions will cause the application to crash.

The most popular and effective strategy for maintaining ABI stability is the PIMPL pattern, but DART uses a considerable amount of template metaprogramming, to the extent that I'm skeptical whether we could reasonably apply the PIMPL pattern to DART in any normal way.

One mitigation strategy is to defer any ABI-breaking changes until a major version increment. I think our ABI-breaking changes are rare enough that this would be an acceptable solution (at least to me).

Alternatively, I might be able to come up with a way to use the Composite class to give us a PIMPL-like way to delay ABI breakages. I'm not entirely sure how plausible this is, though.

I think this is particularly important because of DART's use as a toolkit. It's very likely that different plugins will want to use DART independently of each other for various reasons (e.g. a planning plugin might want to use the forward/inverse kinematics, while a controller plugin might want to use the inverse dynamics, and a physics engine plugin uses the forward dynamics). These plugins might be built and distributed against different versions of DART, especially in the case that a developer needs to make a plugin that uses a new feature of DART that wasn't available in the version that the other plugins were built and distributed for.

The easiest mitigation strategy would be to include the minor version number in the inline namespace, and to install headers to directories that are specific to major+minor version. I think this would be too blunt of an instrument, and it would prevent plugins that are built using different minor versions of DART from communicating using DART symbols.

mxgrey avatar Mar 08 '18 00:03 mxgrey

I know this is still just a proposal, but I wanted to note that I think dart's ABI was broken by https://github.com/dartsim/dart/commit/68aef6b6d66d3f032971752b3f30c77aa13b3c7c#diff-b521b0929fd5887e9bd58eebad913cef (conversion of createShapeNode functions from library to template).

scpeters avatar Apr 24 '18 04:04 scpeters

Related to this concern, an ABI breakage caused some headache here: https://bitbucket.org/ignitionrobotics/ign-physics-release/pull-requests/5/

One thing that might help this issue is allowing minor versions of dartsim to be installed side-by-side.

mxgrey avatar May 21 '19 04:05 mxgrey

One mitigation strategy is to defer any ABI-breaking changes until a major version increment. I think our ABI-breaking changes are rare enough that this would be an acceptable solution (at least to me).

This consensus is realistic to me. We might need a tool to check the ABI compatibility for every change within the same minor version.

One thing that might help this issue is allowing minor versions of dartsim to be installed side-by-side.

I'm a bit hesitated to this solution because it could be too verbose.

jslee02 avatar May 21 '19 06:05 jslee02

I'm a bit hesitated to this solution because it could be too verbose.

Yeah, that's understandable.

mxgrey avatar May 21 '19 06:05 mxgrey