Define a trait for Plugins and allow them to be dynamically loaded or statically linked #114
Conversation
…declaration macro
| @@ -2975,6 +2975,14 @@ dependencies = [ | |||
| "zenoh_backend_traits", | |||
| ] | |||
|
|
|||
| [[package]] | |||
| name = "zenoh-plugin-trait" | |||
| version = "0.1.0-dev" | |||
There was a problem hiding this comment.
Version should be the same as zenoh (0.5.0-dev) since we will release all together.
| # | ||
| [package] | ||
| name = "zenoh-plugin-trait" | ||
| version = "0.1.0-dev" |
There was a problem hiding this comment.
Version should be the same as zenoh (0.5.0-dev) since we will release all together.
There was a problem hiding this comment.
Good point, although I've removed zenoh when I realized it was causing a dependency cycle when I started working on integrating it into zenoh itself
| "Julien Enoch <julien@enoch.fr>", | ||
| "Olivier Hécart <olivier.hecart@adlinktech.com>", | ||
| "Luca Cominardi <luca.cominardi@adlinktech.com>", | ||
| ] |
| /// To signal that your plugin is incompatible with a previously instanciated plugin, return `Err`, | ||
| /// Otherwise, return `Ok(Self::compatibility())`. | ||
| /// | ||
| /// By default, a plugin is non-reentrant to avoir reinstanciation if its dlib is accessible despite it already being statically linked. |
|
|
||
| /// For use with dynamically loaded plugins. Its size will not change accross versions, but its internal structure might. | ||
| /// | ||
| /// To ensure compatibility, its size and alignment must allow `size_of::<Result<PluginVTable, PluginVTableVersion>>() == 64` (one cache line). |
There was a problem hiding this comment.
size_of::<Result<PluginVTable, PluginVTableVersion>> or size_of::<Result<PluginVTableInner, PluginVTableVersion>> ?
There was a problem hiding this comment.
PluginVTable, the goal is for the size of the outer type to be fixed even if we change the size of the inner one (by adding more functions to the vtable for example).
The constant should let us know that the pointers may not point to the same things, but I'm afraid of what would happen if the load_plugin function returns something bigger (or smaller) than what is expected by the caller.
| __padding: [u8; PADDING_LENGTH], | ||
| } | ||
| const PADDING_LENGTH: usize = | ||
| 64 - std::mem::size_of::<Result<PluginVTableInner, PluginVTableVersion>>(); |
There was a problem hiding this comment.
Since the PluginVTablePadding is used along PluginVTableInner within PluginVTable, I don't understand why its length depends on Result<PluginVTableInner, PluginVTableVersion> rather than just PluginVTableInner. Can you explain ?
There was a problem hiding this comment.
It feels a bit less "magic" to set the padding length as "a cache line size minus the size of the type that actually interests us".
Result should use an 8 bytes marker due to align_of::<PluginVTable>() being 8 (and the same as align_of::<PluginVTableInner>()), but that might change (for example if it finds a niche to exploit or realizes that the first bytes are non-null for Ok and null of Err in a few compiler updates), so I'd rather use the Result type to ensure that type has consistent size.
There was a problem hiding this comment.
There is a crate that does that: https://docs.rs/cache-padded/1.1.1/cache_padded/. I suggest to use this crate that is already widely used.
…o abi_stable from here
JEnoch
changed the title
Opening this PR as an RFC. When approved, I'll convert the existing plugins.