Entt: Allow export of used Family template instantiation for shared libraries

[skypjack]

This is a memo/follow-up of a PR made by @DonKult (see #107 for more details).

The fact is that I don't like much the way it is implemented in the PR, mainly because it adds a burden on the project in order to maintain the (let me say) _export stuff_ in future.
It doesn't mean that the implementation is inherently wrong or that I don't want to find a solution. My goal is to provide the minimum support to allow users to use EnTT when dealing with shared libraries, while keeping low the impact on the project (both in terms of changes and code to maintain).

The original author proposed its own version (see the PR for all the deatils) and mentioned also GENERATE_EXPORT_HEADER from cmake as an alternative approach.
If someone with more experience on the subject wants to participate and propose other ways, it is certainly welcome.

I'll leave the issue open for a few days, so as to try to find a way to do it that _works_ (for some definitions of _works_).

---

Of course, I invite @DonKult to actively participate to this discussion. :-)

[skypjack]

My plan so far is to create a dedicated file export.h within config where to put all the instantiations. They come along with by the macro ENTT_EXPORT as suggested by @DonKult in his PR.
Users will be in charge to set it by using cmake or the way they prefer, so as to move on the other side of the border this part. Something like:

#define ENTT_EXPORT __attribute__((visibility("default")))

I'm quite sure I'll forgot to put the next instantiation in the dedicated file in future, whenever I'll use again a family, so... Maintaining i maintaining, well, I'll do my best.
At least, this could be a good compromise probably. Comments?

[skypjack]

Branch experimental contains something along the line I mentioned above. The idea is that this way we won't export forcefully something even when users don't want to do that.
The ENTT_EXPORT macro moves part of the burden on users and gives them more flexibility on the other side.
As I said, probably the best compromise.

I must shutdown my laptop now. If no feedback come until tomorrow, I'll proceed with merging this stuff on master and then I'll close the issue.

[DonKult]

I don't think you can do much in the multiple libraries exporting EnTT case as the solution likely involves one or more of symbol versioning, extern or weak symbols to work in the cases it doesn't work by (more or less) accident.

So perhaps you are right in not wanting to deal with this too centrally. Before seeing your changes I thought it might be best to just move this entirely into the test/example and document what to do in this way rather than in the public headers. People who really need this have still a central place to share there wisdom in, while the rest doesn't need to care. Having a wrapping header in your library around the EnTT include isn't that big a problem… and at least Milerius in shiva seems to do it (for naming convention reasons mostly) and I do as well in my experiments (no real reason).

[skypjack]

@DonKult So, you're suggesting to get rid of the changes, document them and offer an example for those interested, right? No longer an export.h file, if I got it correctly.

[skypjack]

Updated README. Let me know if I wrote something wrong. Thanks.

[Milerius]

Ok fuck me I'm retarded I was wrong, @DonKult is in the truth I need to do it this way

[skypjack]

@Milerius I added a few more to the README. Let me know if you think they are enough for a new user that want to do the same. Thanks.

[Milerius]

You should probably consider the CMake generated things and exporting things like in the original commit, make more sense since is the common idiom to deal with shared library, but is your call, so if you want to keep it like that the problem is that we need to care about the include order I think no ?

[skypjack]

@Milerius Uhm... I didn't get you. What do you suggest to add to the README?

[Milerius]

The original author proposed its own version (see the PR for all the deatils) and mentioned also GENERATE_EXPORT_HEADER from cmake as an alternative approach.
If someone with more experience on the subject wants to participate and propose other ways, it is certainly welcome.

For me use GENERATE_EXPORT_HEADER is the right solution.
I used it long time ago in a project and was great !

This is available on master ? the export.h header, etc ? because i didn't see it !

[Milerius]

https://github.com/skypjack/entt/commit/d71d278b1677defb2c99ec9d782b2879cd16850c#diff-d3dbab5713a595b18a7a4607e050a895

doesn't seems to be merged ?

[skypjack]

@Milerius It isn't. We agreed on the fact that it doesn't worth it and adding a note in the README was enough for users interested in the topic.

[Milerius]

@skypjack Hmmm, the note that you add, didn't solve at all the problem U_u ?

namespace entt {
      template class __attribute__((visibility("default"))) Family<struct InternalRegistryTagFamily>;
      template class __attribute__((visibility("default"))) Family<struct InternalRegistryComponentFamily>;
      template class __attribute__((visibility("default"))) Family<struct InternalRegistryHandlerFamily>;
}

This kind of thing should be done in the library side i think

[skypjack]

@Milerius Granted. What did I leave out?

[Milerius]

@skypjack let me reformulate: this solve the problem, but why the user has to provide it, i think it's better if you provide it as @DonKult pull request, no ?

If one day you add a family, you will have to notice user to take care of it in shared libraries ? doesn't seems to be a long term solution

[skypjack]

@Milerius Mainly because of the maintenance burden that I'd like to avoid and because we cannot provide a _one-fits-all_ solution.
Should I hard-code the export stuff within a bunch of ifdef as suggested with the PR or should I use a cmake based solution that could not work for some users? You know, there isn't _the right approach_, here. Everybody has his own build system and it makes sense that users just do it the way they prefer. With cmake is a matter of minutes, not much work user side.

[Milerius]

@skypjack it's ok ! I think your note in the readme is clear !

[skypjack]

@Milerius Out of curiosity, did you try it also with multiple shared libraries used together?

[Milerius]

@skypjack Yes, doesn't work because of the family class !

let me explain why:

cpp namespace entt { template class __attribute__((visibility("default"))) Family<struct InternalRegistryTagFamily>; template class __attribute__((visibility("default"))) Family<struct InternalRegistryComponentFamily>; template class __attribute__((visibility("default"))) Family<struct InternalRegistryHandlerFamily>; }
```cpp
namespace entt {
template class __declspec(dllexport) Family;
template class __declspec(dllexport) Family;
template class __declspec(dllexport) Family;
}

According to the windows documentation:

https://docs.microsoft.com/en-us/cpp/cpp/using-dllimport-and-dllexport-in-cpp-classes

dllexport Classes

When you declare a class dllexport, all its member functions and static data members are exported. You must provide the definitions of all such members in the same program. Otherwise, a linker error is generated. The one exception to this rule applies to pure virtual functions, for which you need not provide explicit definitions. However, because a destructor for an abstract class is always called by the destructor for the base class, pure virtual destructors must always provide a definition. Note that these rules are the same for nonexportable classes.

If you export data of class type or functions that return classes, be sure to export the class.

The problem when we export the Family is that:

```cpp

/**
 * @brief Dynamic identifier generator.
 *
 * Utility class template that can be used to assign unique identifiers to types
 * at runtime. Use different specializations to create separate sets of
 * identifiers.
 */
template<typename...>
class Family {
    static std::atomic<std::size_t> identifier; // This will be exported

    template<typename...>
    static std::size_t family() ENTT_NOEXCEPT {
        static const std::size_t value = identifier.fetch_add(1); 
      // This will never be exported, and value will not be exported, result that 
      // if you use ComponentA in a dll and ComponentA in the main program, this will increment identifier
        return value;
    }

public:
    /*! @brief Unsigned integer type. */
    using family_type = std::size_t;

    /**
     * @brief Returns an unique identifier for the given type.
     * @return Statically generated unique identifier for the given type.
     */
    template<typename... Type>
    inline static family_type type() ENTT_NOEXCEPT {
        return family<std::decay_t<Type>...>();
    }
};


template<typename... Types>
std::atomic<std::size_t> Family<Types...>::identifier{};


}

[Milerius]

I come with another solution that solve my problem in my case without Virtual interfaces, without boilerplate code, or dllexport/import:

namespace shiva::entt
{
    template <typename Component>
    static inline void init_component(shiva::entt::entity_registry &registry, shiva::entt::entity_registry::entity_type entity) noexcept
    {
        registry.type<Component>();
        if constexpr (std::is_default_constructible_v<Component>)
            registry.assign<Component>(entity);
    }

    template <typename Event>
    static inline void init_event(shiva::entt::dispatcher &dispatcher) noexcept
    {
        static_assert(std::is_default_constructible_v<Event>, "Event must be default constructible");
        dispatcher.sink<Event>();
    }

    template <typename ... Types>
    static inline void init_components(shiva::entt::entity_registry &registry, shiva::entt::entity_registry::entity_type entity,
                         meta::type_list<Types...>)
    {
        (init_component<Types>(registry, entity), ...);
    }

    template <typename ... Types>
    static inline void init_events(shiva::entt::dispatcher &dispatcher, meta::type_list<Types...>)
    {
        (init_event<Types>(dispatcher), ...);
    }

    static inline void init_library(shiva::entt::entity_registry &registry, shiva::entt::dispatcher &dispatcher)
    {
        auto id = registry.create();
        init_events(dispatcher, shiva::event::common_events_list{});
        init_components(registry, id, shiva::ecs::common_components{});
        registry.destroy_entity(id);
        registry.reset();
    }
}

template <typename SystemDerived, typename TSystemType>
    class system : public base_system
    {
        template <typename T>
        using is_kind_system = std::is_same<TSystemType, T>;
    public:
        template <typename ...Args>
        explicit system(Args &&...args) noexcept : base_system(std::forward<Args>(args)...),
                                                   log_{shiva::log::stdout_color_mt(SystemDerived::class_name())}
        {
            if (this->is_a_plugin())
                shiva::entt::init_library(entity_registry_, dispatcher_);
        }

// Example of a meta::list

namespace shiva::ecs
{
    using common_components = meta::type_list<layer_1,
        layer_2,
        layer_3,
        layer_4,
        layer_5,
        layer_6,
        layer_7,
        layer_8>;
}

Calling init_library in the DLL's, and in the main program will regulate the static variable of family for the dll's and for the main program. It's not painfull, and it's maintainable i just have to add a component or a event in the meta::list and then my program is regulate, i didn't cross the limit anymore

[skypjack]

@Milerius You're forcing the same order while generating identifiers for components that are shared by the two sides of the boundary. It makes sense. :smile:

[Milerius]

@skypjack Totally, i think you can put a little not for this approach in a readme, i didn't think the current one work since the static const std::size_t value will always be reset

in my approach there is two thing's that is not so great:

• Default constructability
  
  
  
  • More Compilation time
  
  

but:

• Keep Performance from EnTT library as his maximum potential
• Maintainable

[skypjack]

@Milerius

i didn't think the current one work since the static const std::size_t value will always be reset

I didn't get it. @DonKult shown that an executable and a shared library nicely work together by exporting the family classes. The problem was with multiple shared libraries and I can see it. Did you experience problems with a single shared library instead?

[Milerius]

entt_shared is linked to all the dll and to the main program
init_library is in the entt_shared dll

now i have:

[skypjack]

@Milerius Probably it's worth it asking on SO if someone can suggest an alternative approach for the family class that doesn't slow down performance and that works across boundaries at the same time. I think I'll do it.

[Milerius]

@skypjack Yeah ! give us the link if you do something like this !

[skypjack]

@Milerius sure - https://stackoverflow.com/q/51332851/4987285
Let's see if someone more clever than me pops up with a good suggestion.

[Milerius]

@skypjack i shared the post on cpplang.sh

[Milerius]

https://github.com/Manu343726/ctti

apparently someone say it's a good approach

but i think it's not an incremental type id like you want

[skypjack]

@Milerius It looks like it uses internally much more code to do the same thing that EnTT does with its hashed string. :-)
Btw, so far it seems that the best solution is to provide users with a way to set their own family class on types, so that they can work around the limitations of static stuff with a custom solution that has lower performance if required.
As long as an user provided family has the same API (namely it exposes a type member function) and can generate a sequence of identifiers that starts from 0, everything should work just fine.

[skypjack]

The other way around that could work is a mixed compile-time plus runtime registry, so that types shared across boundaries can get fixed identifiers at compile-time and only the executable is allowed to generate new identifiers.

[Milerius]

@skypjack If you allow the first solution everything become so easy:

template<typename T, typename MetaList>
class Family {
    using List = MetaList;

    template<typename T>
    static constexpr std::size_t family() ENTT_NOEXCEPT {
        return  meta::list::Position<List, T>();
    }

public:
    /*! @brief Unsigned integer type. */
    using family_type = std::size_t;

    /**
     * @brief Returns an unique identifier for the given type.
     * @return Statically generated unique identifier for the given type.
     */
    template<typename... Type>
    inline static family_type type() ENTT_NOEXCEPT {
        return family<std::decay_t<Type>...>();
    }
};

            template <typename List, typename ToFind>
            struct Position;

            template <typename T, typename ...ListTypes, typename ToFind>
            struct Position<meta::type_list<T, ListTypes...>, ToFind> :
                std::integral_constant<size_t, 1 + Position<meta::type_list<ListTypes...>, ToFind>()>
            {
            };

            template <typename ...ListTypes, typename ToFind>
            struct Position<meta::type_list<ToFind, ListTypes...>, ToFind> : std::integral_constant<size_t, 0>
            {
            };

[skypjack]

@Milerius I see. You're injecting something conceptually similar to the ident class, that is exactly what the registry would do under the hood in the second solution.
At the end of the day, to rely on our dear compile-time is the best solution in this case. ;-)

Btw I'm at the swimming pool right now. I think I won't allow anything at least until tomorrow. :-)

[Milerius]

@skypjack will see monday i think, tomorrow is France-Croatie i canno't miss this since i'm french <3

[skypjack]

Let's continue on #114 anyway.