Consider "-inl.h" design pattern to reduce compile time for templated code

[sherm1]

After PR #154 lands, most FCL code is templatized by scalar type. In normal use it will almost always be instantiated with double (maybe float). The more elaborate instantiations needed for autodifferentiation will be rare, so most user code needs only declarations for the T=double case and doesn't need to have access to the definitions of the templatized methods, which are expensive to compile.

In Drake we use this design pattern to hide the definitions from compilation units that don't need them. Then we explicitly instantiate the most common T=double case in library code. So user code normally includes blah.h; code that needs to make its own instantiation includes blah-inl.h.

We should consider this for FCL also to lighten the compilation load, reduce code bloat, and hide implementations when possible.

[jslee02]

I like this idea! I've been looking for a way of reducing the compilation time after I experienced dramatically increased compilation time with #154.

I proposed minimizing exposing API to users using detail directory. Accordingly, I would prefer to have the implementation files in detail directory. The file name could be either of the same as the declaration file or the declaration file name with a suffix. For the suffix, I would prefer -impl.h because -inl.h sounds like for inline functions. How do you think?

One concern with this design pattern is that FCL wouldn't be a header-only project, which I was intended for the next work. But now I think reducing compilation time is more worth and don't have a compelling reason for making FCL header-only project.

[sherm1]

Yeah, I agree -impl.h makes more sense than -inl.h.

I think with this design pattern FCL could be used header-only, if people use the -impl.h headers. But most users would probably be glad to link instead to avoid recompiling all that code with every use.

[jslee02]

We could consider explicitly instantiating for T=fcl::real rather than T=double, and aliasing fcl::real as double by default. This would allow us to build FCL instantiated with different type by simply switching the fcl::real to the different type.

[sherm1]

Actually for explicit instantiations it wouldn't be unreasonable to simply instantiate for float and double both if those are the only common ones. Since the code would just exist once for each in the library the overhead would be minimal. Then I think there would be no need for fcl::real and people wouldn't have to try to coordinate that with whatever other packages they are using.

[sherm1]

@jslee02, Jeremy Nimmer asserts that he used -inl.h in Drake because it is sort of an industry standard despite its somewhat-wrong connotation. See RobotLocomotion/drake#3141.

[jslee02]

If it's an industry standard then I'm okay with it. I don't have a strong preference here.

[scpeters]

I don't like to argue over naming, but if it's an industry standard and used in open source software, then it should be easy to find examples that use this convention.

[sherm1]

Googling for "template inl.h instantiation" does pull up a lot of uses, including a mention in the Google Style Guide, basically to say "don't do this" (but they aren't talking about this specific problem). impl.h definitely doesn't get the same amount of press.

[scpeters]

Searching the ubuntu packages database, I see it in nodejs, unity, gtest, and mongodb. So it is in use in some well-known projects.

[jslee02]

Interestingly I got 1,260 results for -impl.h while got 762 results for -inl.h.

[jslee02]

Anyways, I created a pull request for this. Feedback is welcome!

[sherm1]

Interestingly I got 1,260 results for -impl.h while got 762 results for -inl.h.

I saw that too but then when I looked closer Google had apparently dropped the ".h" and matched lots of things containing the word "implementation"!

[jslee02]

I believe this is addressed by #165.