Reworked plugin database

[undisputed-seraphim]

This merge request seeks to organize the plugin interface of OpenRAVE. The functions that can be called on a plugin are now collected into a virtual class that all plugins must inherit from and override the pure virtual methods.

The symbols that needs to be loaded from a plugin shared object is reduced to just a single C call - RavePlugin* CreatePlugin();, eliminating the need to check for the existence of multiple symbols. Previous implementation required a thread to lazy load libraries as checking for multiple symbols was time consuming; however it still eager-loaded the OpenRaveGetPluginAttributes function. Here since there's only one function to load, and we already have the time budget to eager-load one function, there is no longer a need for the extra thread, and all the supporting mutexes and condition_variables surrounding that thread.

The previous RegisteredInterface object has also been folded into the plugin object class by recognizing it as a special subclass of plugins that don't inhabit a separate binary. This eliminates the need for a different set of code paths to handle them - just create a VirtualPlugin object and set the fields to what was previously being set in RegisteredInterface.

All the plugins have been updated to reflect the changes made to the openrave/plugin.h header. Some plugins have interfaces that don't have an implementation; those interfaces have been commented out (linking fails otherwise).

The advantages of this merge request is that plugin and interface creation code are now neatly segregated into a class, and statically compiling in plugins will not require large amounts of code duplication.

I recognize that this is a rather significant change that breaks existing conventions. Effort has been made to ensure that internal libraries are also updated, tested, and fixed in sync. I may have some unknown unknowns, kindly review.

[rdiankov]

@undisputed-seraphim This way might be slower now since dlopen has to process more symbols in the SO. Please do some perf experiments with starting the plugin database and getting all plugin info comparing your way and the current production way. thanks

[undisputed-seraphim]

I timed the changes in two ways. For every test, OpenRave was able to find 14 plugins, the ones bundled in this repository as well as elsewhere in our system.

1. Using time openrave --listplugins. Ran it for 5 times each, and the final column is the average.

Before

real(s) |0.056 |0.063 |0.055 |0.085 |0.062 | 0.0642
user(s) |0.030 |0.038 |0.027 |0.044 |0.027 | 0.0332
sys(s)  |0.018 |0.018 |0.020 |0.033 |0.027 | 0.0232

After

real(s) |0.070 |0.065 |0.069 |0.065 |0.070 | 0.0678
user(s) |0.039 |0.042 |0.037 |0.042 |0.037 | 0.0394
sys(s)  |0.023 |0.016 |0.025 |0.016 |0.026 | 0.0212

New changes are slower by about 0.002 to 0.006 seconds.

2. Using high_resolution_clock to time the execution of running RaveInit and RaveCreateEnvironment. Ran each test 3 times, final column is the average.

Times are in nanoseconds.

Before

1130700
1120776
1097157
Average: 1116211

After

1115112
1111894
1114423
Average: 1113810 (-2401 nanoseconds)

New changes are faster by 2401 nanoseconds (0.002401 milliseconds)

I contend that this is negligible and furthermore only performed once per invocation of RaveCreateEnvironment.

On the other hand, this changes brings us some benefits:

• As the plugin is now represented by an object, we can use RAII to correctly and safely release any memory (or other resource) required by the plugin.
• As the lifetimes of values and variables is now controlled by the object, we don't have to use hacky null-checks and workarounds for static values such as these.
• Due to the above point, we can avoid static initialization fiasco, as now it is controlled by the object in a well-defined way.
• Compilation will fail if required functions are not implemented - as opposed to failing only at runtime, and requiring a rebuild.
• The plugin entrypoint is now represented by a single C-compatible linkage, and is therefore cross-platform and also language agnostic if anyone wants to create bindings.
• New libopenrave0.101.so binary is slightly smaller (old: 4235680 bytes, new: 4223392 bytes; smaller by exactly 12 kilobytes!)
• It enables us to implement a fully static OpenRAVE library with all plugins embedded in it for more performance gains.

[cielavenir]

I'm personally interested in COM (where all interfaces inherit IUnknown https://github.com/jinfeihan57/p7zip/blob/master/CPP/Common/MyWindows.h#L119)

[cielavenir]

In this way the only exported symbol is "CreateObject", so it costs less for ld-linux (symbol resolver)

[rdiankov]

@undisputed-seraphim Can you send the c++ code you used to do perf tests? thanks!

[undisputed-seraphim]

@undisputed-seraphim Can you send the c++ code you used to do perf tests? thanks!

For temporary testing, I merged the branch liboon/tests, and built and executed the tests. The test executable is named openrave_tests

liboon/tests is a branch I am using to develop a set of tests written in C++.

[undisputed-seraphim]

@rdiankov I've written another benchmark that produces a more stark difference.

#include <chrono>
#include <iostream>
#include <vector>
#include <openrave/openrave.h>
#include <openrave-core.h>

constexpr int ITERATIONS = 1000;

int main() {
    std::vector<std::chrono::nanoseconds> deltas = {};
    deltas.resize(ITERATIONS);
    for (int i = 0; i < ITERATIONS; ++i) {
        auto start = std::chrono::high_resolution_clock::now();
        OpenRAVE::RaveInitialize(true);
        OpenRAVE::RaveDestroy();
        deltas[i] = (std::chrono::high_resolution_clock::now() - start);
    }
    uint64_t average_ns = 0;
    for (const auto& delta : deltas) {
        average_ns += delta.count();
    }
    average_ns /= ITERATIONS;
    double average_ms = average_ns / 1'000'000.0;
    std::cout << average_ns << " nanoseconds (" << average_ms << " milliseconds), on average, across 1000 iterations" << std::endl;
    return 0;
}

Linked against current production: 807368 nanoseconds (0.807368 milliseconds) on average across 1000 iterations. Linked against my changes: 244185 nanoseconds (0.244185 milliseconds), on average across 1000 iterations. Indicating a ~3.5 times speedup.

[rdiankov]

@undisputed-seraphim For the c++ benchmark, please add getting the list of plugin information. thanks

[undisputed-seraphim]

@rdiankov

#include <chrono>
#include <iostream>
#include <array>
#include <openrave/openrave.h>
#include <openrave-core.h>

constexpr int ITERATIONS = 1000;

int main() {
    std::list<std::pair<std::string, OpenRAVE::PLUGININFO>> plugins;
    std::array<std::chrono::nanoseconds, ITERATIONS> deltas;
    for (int i = 0; i < ITERATIONS; ++i) {
        plugins.clear();
        auto start = std::chrono::high_resolution_clock::now();
        OpenRAVE::RaveInitialize(true);
        OpenRAVE::RaveGetPluginInfo(plugins);
        OpenRAVE::RaveDestroy();
        deltas[i] = (std::chrono::high_resolution_clock::now() - start);
    }
    std::cout << "Retrieved information about " << plugins.size() << " plugins." << std::endl;
    uint64_t average_ns = 0;
    for (const auto& delta : deltas) {
        average_ns += delta.count();
    }
    average_ns /= ITERATIONS;
    double average_ms = average_ns / 1'000'000.0;
    std::cout << average_ns << " nanoseconds (" << average_ms << " milliseconds), on average, across 1000 iterations" << std::endl;
    return 0;
}

Output (production):

Retrieved information about 15 plugins.
783248 nanoseconds (0.783248 milliseconds), on average, across 1000 iterations

Output (newPlugin):

Retrieved information about 15 plugins.
251012 nanoseconds (0.251012 milliseconds), on average, across 1000 iterations

[rdiankov]

@undisputed-seraphim Awesome, good job! So once I merge this, we need to also change all our internal plugins in controllercommon and planningcommon right? Can you prepare merge requests for that? Thanks!

[undisputed-seraphim]

@rdiankov I have prepared merge requests for those and assigned them to you.

[rdiankov]

woot, congrats~