Version 2.0.0: generation of a library with default implementation

[jcarpent]

It has been suggested in #570 to generate a dynamic library that contains the compiled version of a default implementation (e.g. Model, Data and all the default algorithms).

This issue is opened to discuss this implementation and the technical details.

[jcarpent]

I have checked this part, and it seems that we need to compile with -std=c++11 to be able to generate templated code in a library. We need to use the keyword externe in the following way:

extern template class se3::SE3Tpl<double>;

to generate the whole SE3 class for double.

[jcarpent]

I think using C++11 will help us to improve the interoperability of Pinocchio. I mean by that code compiled with C++98 standard will be able to be linked to this C++11 library while the reverse is not true.

[jmirabel]

You do not need C++ 11 to do that. As an example, see here.

template class se3::SE3Tpl <double>;

Do you know what extern keyword adds in this case ?

[jcarpent]

I checked and your version is not necessarily handled by all the compilers. Here, the extern mentions that the template has been generated in another .o file. Your version will generate the template only for the current .o file, not for the others.

So if you set your example in several .cpp files, they will also generate the template and duplicate the code.

[jcarpent]

So in one cpp file, you will set:

template class se3::SE3Tpl<double>;

and in the orders, you just need to say:

extern template class se3::SE3Tpl<double>;

[jmirabel]

Well, if other C++ files only have the declaration and not the definition of se3::SE3Tpl, they won't generate a new se3::SE3Tpl<double>.

[jmirabel]

But, yes, this has only been checked with gcc. I didn't try with other compilers.

[jcarpent]

This means that we need to really split declaration from definition in our classes, which is a tedious work.

[jmirabel]

I agree. I check how FCL or PCL do. I know understand the piece of code which achieves this.

They add at the beginning of each definition file the line you mentioned (extern template class Foo<0>;). I wonder what happen if you compile a file that contains this line and you do not link to the library that provides its implementation: Will the compiler compile Foo<0> or fail with a linking error ?

If it compiles Foo<0>, we merely add the extern declaration for all class and play with CMake and pkg-config ! Otherwise, we must surround the extern declaration with a #ifdef PINOCCHIO_LINK_TO_LIBRARY ... #endif.

[nmansard]

Indeed, I do not see any alternative to that: even if you manage to pre-compile a specialized version of the template, you still need to have access to the declaration in the 3rd-party code, is it not?

Re-organizing would have the advantage to also enable faster compilation in 3rd parties, that would then only include declaration in their header, and some definitions in their sources.

But this is a huge work, both in the definition of the methodology and in the execution of it. I suggest to decouple it from 2.0.0. What we could do is to anticipate in the way headers should be included in 2.0.0, so that changing the lib organization afterward does not change the API. But even that seems a big package of work.

Are we clear about how this problem is tackle in Eigen? I suppose we have a lot to learn from the Eigen team.

On 10/30/2018 10:34 AM, Justin Carpentier wrote:

This means that we need to really split declaration from definition in our classes, which is a tedious work.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stack-of-tasks/pinocchio/issues/571#issuecomment-434231341, or mute the thread https://github.com/notifications/unsubscribe-auth/AAb-CI2FBhbNfKk01q0RiSZh48TCwvrLks5uqB0WgaJpZM4X_2gm.

[jmirabel]

Indeed, I do not see any alternative to that: even if you manage to pre-compile a specialized version of the template, you still need to have access to the declaration in the 3rd-party code, is it not?

No, you do not. At least, it works fine with gcc.

Are we clear about how this problem is tackle in Eigen? I suppose we have a lot to learn from the Eigen team.

I don't think Eigen methodology will help much in this direction. They do not need a clear separation between declaration and definition and they definitely do not have.

[jmirabel]

But this is a huge work, both in the definition of the methodology and in the execution of it. I suggest to decouple it from 2.0.0. What we could do is to anticipate in the way headers should be included in 2.0.0, so that changing the lib organization afterward does not change the API. But even that seems a big package of work.

For the purpose of the library, I do not think you need a clear separation of the declaration and definition. I think it isn't that hard to achieve.

For decreasing the compilation time and provide declaration-only headers, I agree the work is long and tedious. This is not for v2.0.

[jcarpent]

So then let's skip the code generation part for the current release. Anyway, by using the keyword 'extern template', we have to rely on C++11 (which is not bad). Of course, if people still need to compile only with C++98 standard, they will be able to do it through the header-only part.

[jcarpent]

I agree. I check how FCL or PCL do. I know understand the piece of code which achieves this.

They add at the beginning of each definition file the line you mentioned (extern template class Foo<0>;). I wonder what happen if you compile a file that contains this line and you do not link to the library that provides its implementation: Will the compiler compile Foo<0> or fail with a linking error ?

If it compiles Foo<0>, we merely add the extern declaration for all class and play with CMake and pkg-config ! Otherwise, we must surround the extern declaration with a #ifdef PINOCCHIO_LINK_TO_LIBRARY ... #endif.

It will depend if you a partial or a complete definition of your class I guess.

[jmirabel]

Is this going to be in v2 ?

[jcarpent]

It can be, but it requires some refactoring work.

The question is first: do we want to impose C++11 for extern templatekeywords use?

[jmirabel]

What about a incomplete but already better solution of encapsulating all #include <foo.hxx> by some #ifndef PINOCCHIO_USE_LIBRARY ... #endif` and adding the flags to pkg-config file ?

[jcarpent]

I think we should keep .hxx files and just add between an #ifdef the extern template keywords. In this way, the compiler won't compile the template if we refer them through keywords.

In particular, I think only Model, Data and all the algorithms and parsers should be externalized. It should be sufficient for HPP and other frameworks.

@jmirabel What do you think?

[jmirabel]

Fine, go ahead for this.

[nmansard]

Are you sure you can keep the compatibility with C++ 98?

[jcarpent]

Yes. Just people using C++98 won't have access to this feature, as it requires C++11.

[jcarpent]

I will provide a solution to this issue soon.

[nmansard]

Most of the teams using pinocchio are just in the middle of the merge toward pinocchio v2. Please be very careful not to damage the current API in any mean.

[jcarpent]

Yes, it won't be the case. It will just use the C++11 feature. There is no need to change the API to use it. If C++98 is used, then this feature won't be activated and all the code will be compiled as it is the case now.

[jcarpent]

@jmirabel wrote:

I gave a quick trial to template instantiation for C++ >= 11: dbffed2

With this commit, compiling file lib.cc:

#include <pinocchio/algorithm/kinematics.hpp>

using namespace pinocchio;

Eigen::MatrixXd foo()
{
  Model model;
  Data data(model);
  Eigen::MatrixXd q;
}

with the CMakeLists.txt:

cmake_minimum_required(VERSION 3.10)

project(test LANGUAGES CXX)

find_package(pinocchio REQUIRED)

add_library(lib_linked SHARED lib.cc)
target_link_libraries(lib_linked pinocchio::pinocchio)

add_library(lib_not_linked SHARED lib.cc)
target_link_libraries(lib_not_linked pinocchio::pinocchio)
target_compile_definitions(lib_not_linked PRIVATE "-DPINOCCHIO_ENABLE_TEMPLATE_INSTANTIATION=0")

Then, time make lib_linked (using template instantiation) gives:

real 0m6,208s user 0m5,707s sys 0m0,482s

and time make lib_not_linked (using template instantiation) gives:

real 0m8,290s user 0m7,591s sys 0m0,570s

The benefits in terms of compilation time is obvious.

[jcarpent]

Thanks a lot @jmirabel for this attempt and I would say this result was expected. We just need time to handle correctly and completely this task.

I think we should wait for #911 which will impose some major modifications, and then handle this one. @jmirabel Do you agree?

[jcarpent]

By the way, the template instantiation should be automatic for C++11 and more.

[jcarpent]

Done in pinocchio-3x which should be released soon.