Skip to content

Commit

Permalink
Fixing problems with polymorphic serialization
Browse files Browse the repository at this point in the history
  • Loading branch information
@sithhell
sithhell committed Apr 17, 2015
1 parent 17d7721 commit cf73d36
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 58 deletions.
69 changes: 17 additions & 52 deletions hpx/runtime/serialization/detail/polymorphic_intrusive_factory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,17 @@ namespace hpx { namespace serialization { namespace detail

void register_class(const std::string& name, ctor_type fun)
{
map_.emplace(name, fun);
if(name.empty())
{
HPX_THROW_EXCEPTION(serialization_error
, "polymorphic_intrusive_factory::register_class"
, "Cannot register a factory with an empty name");
}
auto it = map_.find(name);
if(it == map_.end())

This comment has been minimized.

Sign in to view

Copy link
@AntonBikineev

AntonBikineev Apr 21, 2015

Contributor

is this check worth it? Doesn't it lie inside emplace?

This comment has been minimized.

Sign in to view

Copy link
@sithhell

sithhell via email Apr 21, 2015

Author Member
{
map_.emplace(name, fun);
}
}

template <class T>
Expand Down Expand Up @@ -157,61 +167,16 @@ namespace hpx { namespace serialization { namespace detail
Class, BOOST_PP_STRINGIZE(Class)) \
/**/

namespace hpx { namespace serialization { namespace detail {

struct unique_suffix_for_template_name: boost::noncopyable
{
static unique_suffix_for_template_name& instance()
{
hpx::util::static_<unique_suffix_for_template_name> instance;
return instance.get();
}

std::string get_new_suffix()
{
const uint32_t temp = ++suffix;
return util::safe_lexical_cast<std::string>(temp);
}

private:
unique_suffix_for_template_name():
suffix(0U)
{}

friend struct hpx::util::static_<unique_suffix_for_template_name>;
boost::atomic<boost::uint32_t> suffix;
};

template <class T>
std::string get_unique_suffix_for_template_name()
{
static std::string suffix =
unique_suffix_for_template_name::instance().get_new_suffix();
return suffix;
}

}}}

#include <hpx/config/warnings_suffix.hpp>

#define HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE_WITH_NAME(Class, Name) \
HPX_SERIALIZATION_POLYMORPHIC_WITH_NAME(Class, (Name + \
::hpx::serialization::detail::get_unique_suffix_for_template_name<Class>())) \
#define HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE(Class) \
HPX_SERIALIZATION_POLYMORPHIC_WITH_NAME( \
Class, util::type_id<Class>::typeid_.type_id();) \

This comment has been minimized.

Sign in to view

Copy link
@AntonBikineev

AntonBikineev Apr 21, 2015

Contributor

Thomas, does this mean that we will always get assertion error when running hpx application compiled with typeid incompatible compilers?

This comment has been minimized.

Sign in to view

Copy link
@K-ballo

K-ballo Apr 21, 2015

Member

Hopefully! Which today means mixing Linux/Windows (and possibly Android, IIRC all their typeids are empty). We have discussed the possibility of porting non-Itanium ABIs to Itanium mangled scheme to have portable type names, but that will take a lot of research and work.

However, in the meantime, it means that mixing applications compiled with typeid compatible compilers will do the right thing now.

This comment has been minimized.

Sign in to view

Copy link
@sithhell

sithhell Apr 21, 2015

Author Member

By default, the code will fall back to typeid based identification. This proofed to be valuable for quick prototyping. If you run in a heterogeneous environment, you absolutely want to turn automatic registration of. The assert only fires if this function can not see the declaration.
/**/

#define HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE_WITH_NAME_SPLITTED(Class, Name) \
HPX_SERIALIZATION_POLYMORPHIC_WITH_NAME_SPLITTED(Class, (Name + \
::hpx::serialization::detail::get_unique_suffix_for_template_name<Class>())) \
/**/

#define HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE(Class) \
HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE_WITH_NAME( \
Class, BOOST_PP_STRINGIZE(Class) BOOST_PP_STRINGIZE(__COUNTER__)) \
/**/

#define HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE_SPLITTED(Class) \
HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE_WITH_NAME_SPLITTED( \
Class, BOOST_PP_STRINGIZE(Class) BOOST_PP_STRINGIZE(__COUNTER__)) \
#define HPX_SERIALIZATION_POLYMORPHIC_TEMPLATE_SPLITTED(Class) \
HPX_SERIALIZATION_POLYMORPHIC_WITH_NAME_SPLITTED( \
Class, util::type_id<T>::typeid_.type_id();) \
/**/

#endif
59 changes: 54 additions & 5 deletions hpx/runtime/serialization/detail/polymorphic_nonintrusive_factory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <hpx/util/static.hpp>
#include <hpx/util/demangle_helper.hpp>
#include <hpx/traits/polymorphic_traits.hpp>
#include <hpx/traits/needs_automatic_registration.hpp>

#include <boost/noncopyable.hpp>
#include <boost/unordered_map.hpp>
Expand All @@ -23,6 +24,25 @@

namespace hpx { namespace serialization { namespace detail
{
template <typename T>
char const* get_serialization_name()
#ifdef HPX_DISABLE_AUTOMATIC_SERIALIZATION_REGISTRATION
;
#else
{
/// If you encounter this assert while compiling code, that means that
/// you have a HPX_REGISTER_ACTION macro somewhere in a source file,
/// but the header in which the action is defined misses a
/// HPX_REGISTER_ACTION_DECLARATION
BOOST_MPL_ASSERT_MSG(
traits::needs_automatic_registration<T>::value
, HPX_REGISTER_ACTION_DECLARATION_MISSING
, (T)
);
return util::type_id<T>::typeid_.type_id();
}
#endif

struct function_bunch_type
{
typedef void (*save_function_type) (output_archive& , const void* base);
Expand All @@ -49,7 +69,15 @@ namespace hpx { namespace serialization { namespace detail
void register_class(const std::string& class_name,
const function_bunch_type& bunch)
{
map_.emplace(class_name, bunch);
if(class_name.empty())
{
HPX_THROW_EXCEPTION(serialization_error
, "polymorphic_nonintrusive_factory::register_class"
, "Cannot register a factory with an empty name");
}
auto it = map_.find(class_name);
if(it == map_.end())

This comment has been minimized.

Sign in to view

Copy link
@AntonBikineev

AntonBikineev Apr 21, 2015

Contributor

same for this
map_.emplace(class_name, bunch);
}

// the following templates are defined in *.ipp file
Expand Down Expand Up @@ -107,8 +135,8 @@ namespace hpx { namespace serialization { namespace detail

polymorphic_nonintrusive_factory::instance().
register_class(
typeid(Derived).name(),
bunch
get_serialization_name<Derived>(),
bunch
);
}

Expand Down Expand Up @@ -139,7 +167,7 @@ namespace hpx { namespace serialization { namespace detail

polymorphic_nonintrusive_factory::instance().
register_class(
typeid(Derived).name(),
get_serialization_name<Derived>(),
bunch
);
}
Expand All @@ -161,10 +189,31 @@ namespace hpx { namespace serialization { namespace detail

#include <hpx/config/warnings_suffix.hpp>

#define HPX_SERIALIZATION_REGISTER_CLASS(Class) \
#define HPX_SERIALIZATION_REGISTER_CLASS_DECLARATION(Class) \

This comment has been minimized.

Sign in to view

Copy link
@AntonBikineev

AntonBikineev Apr 21, 2015

Contributor

What's the purpose of splitting it into declaration|definition?

This comment has been minimized.

Sign in to view

Copy link
@sithhell

sithhell Apr 21, 2015

Author Member

One comes into the header, the other into a TU. This potentially speeds up compilation.
namespace hpx { namespace serialization { namespace detail { \
template <> HPX_ALWAYS_EXPORT \
char const* get_serialization_name<Class>(); \
}}} \
namespace hpx { namespace traits { \
template <> \
struct needs_automatic_registration<action> \
: boost::mpl::false_ \
{}; \
}} \
HPX_TRAITS_NONINTRUSIVE_POLYMORPHIC(Class); \

#define HPX_SERIALIZATION_REGISTER_CLASS_NAME(Class, Name) \
namespace hpx { namespace serialization { namespace detail { \
template <> HPX_ALWAYS_EXPORT \
char const* get_serialization_name<Class>() \
{ \
return BOOST_PP_STRINGIZE(Name); \
} \
}}} \
template hpx::serialization::detail::register_class<Class> \
hpx::serialization::detail::register_class<Class>::instance; \
/**/
#define HPX_SERIALIZATION_REGISTER_CLASS(Class) \
HPX_SERIALIZATION_REGISTER_CLASS_NAME(Class, Class) \

#endif
6 changes: 5 additions & 1 deletion hpx/runtime/serialization/string.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ namespace hpx { namespace serialization
void serialize(output_archive & ar, std::basic_string<Char, CharTraits, Allocator> & s, unsigned)
{
ar << s.size();
if(s.empty()) return;
if(s.empty())
{
HPX_ASSERT(s.size() == 0);
return;
}

save_binary(ar, s.data(), s.size() * sizeof(Char));
}
Expand Down

0 comments on commit cf73d36

Please sign in to comment.