Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hpx serialization #1446

Merged
merged 72 commits into from Apr 14, 2015
Merged

Hpx serialization #1446

merged 72 commits into from Apr 14, 2015

Conversation

AntonBikineev
Copy link
Contributor

Some inconsistent stuff still exists. Mostly it's related to polymorphic serialization because current polymorphic serialization mechanism(intrusive and nonintrusive) has to be refined.

sithhell and others added 30 commits October 25, 2014 12:53
additional check in locality constructor.
	modified:   hpx/runtime/parcelset/locality.hpp
Conflicts:
	docs/manual/build_system/cmake_variables.qbk
	examples/quickstart/non_atomic_rma.cpp
	hpx/components/vector/distribution_policy.hpp
	hpx/components/vector/vector_configuration.hpp
	hpx/lcos/broadcast.hpp
	hpx/lcos/fold.hpp
	hpx/lcos/reduce.hpp
	hpx/parallel/algorithms/detail/dispatch.hpp
	hpx/performance_counters/counters.hpp
	hpx/plugins/parcelport/mpi/locality.hpp
	hpx/plugins/parcelport/tcp/sender.hpp
	hpx/runtime/actions/action_support.hpp
	hpx/runtime/agas/request.hpp
	hpx/runtime/components/stubs/runtime_support.hpp
	hpx/runtime/naming/name.hpp
	hpx/runtime/parcelset/decode_parcels.hpp
	hpx/runtime/parcelset/encode_parcels.hpp
	hpx/runtime/parcelset/locality.hpp
	hpx/runtime/parcelset/policies/mpi/sender.hpp
	hpx/serialization/input_container.hpp
	hpx/serialization/output_container.hpp
	hpx/util/basic_binary_iprimitive.hpp
	hpx/util/basic_binary_oprimitive.hpp
	hpx/util/portable_binary_iarchive.hpp
	hpx/util/portable_binary_oarchive.hpp
	hpx/util/serialize_empty_type.hpp
	hpx/util/serialize_sequence.hpp
	hpx/util/tuple.hpp
	src/CMakeLists.txt
	src/components/dataflow/dataflow_component.cpp
	src/components/vector/vector_configuration.cpp
	src/performance_counters/counters.cpp
	src/runtime/agas/big_boot_barrier.cpp
	src/runtime/components/server/runtime_support_server.cpp
	src/runtime/parcelset/parcel.cpp
	src/util/portable_binary_iarchive.cpp
	src/util/portable_binary_oarchive.cpp
@@ -810,7 +814,7 @@ if(HPX_COMPILER_WARNINGS)
hpx_add_compile_flag_if_available(-Werror=uninitialized)
endif()

#hpx_add_compile_flag_if_available(-Werror=missing-field-initializers)
hpx_add_compile_flag_if_available(-Wno-missing-field-initializers)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this warning disabled? It seems it was considered an error before. In case this is actually needed, it seems it should be moved out of the warnings-as-errors section.

@K-ballo
Copy link
Member

K-ballo commented Apr 5, 2015

Is there a way to tell if an Archive type is an archive, and whether it's an input or output one? Or some other way to implement serialization support without ever including a single bit of the serialization library itself?

#include <hpx/util/reinitializable_static.hpp>

#define HPX_SINGLE_ARG(...) __VA_ARGS__
#define HPX_COMMA ,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be the proper place for these macro definitions, nor they seem to be used anywhere either. Are they still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to delete them

@K-ballo
Copy link
Member

K-ballo commented Apr 5, 2015

Nitpick: the placement of braces after namespace is inconsistent across this PR. The dominant style within existing code is:

namespace hpx { namespace whatever // newline
{ // opening brace

I don't think this is explicitly stated anywhere, so I'd appreciate if others would chime in.

}
}}

#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header should include whatever defines boost::intrusive_ptr, even while it's probably already included by serialization/detail/pointer.hpp

@K-ballo
Copy link
Member

K-ballo commented Apr 5, 2015

Some save/load interfaces use input/output_archive as argument, others use a template argument (which I'd rather see renamed to something other than just T). What's the guideline here, when should I pick one or the other?

#include <boost/preprocessor/stringize.hpp>
#include <boost/noncopyable.hpp>
#include <boost/unordered_map.hpp>
#include <boost/atomic/atomic.hpp>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be just <boost/atomic.hpp> for backward compatibility. See it fail with older boost versions here http://hermione.cct.lsu.edu/builders/hpx_gcc_4_6_boost_1_49_debian_x86_64_debug/builds/266/steps/build_core/logs/stdio

@AntonBikineev
Copy link
Contributor Author

@K-ballo I everywhere tried to use template <class Archive> instead of just template <class T>. I think, no guidelines on templated/nontemplated versions are there. It's just easier for user to have Boost.Serialization compatible interface. We can stick to one way inside HPX, though.

@AntonBikineev
Copy link
Contributor Author

Is there a way to tell if an Archive type is an archive, and whether it's an input or output one? Or some other way to implement serialization support without ever including a single bit of the serialization library itself?

I don't see any, if I understand you correctly.

array(const array& rhs):
m_t(rhs.m_t),
m_element_count(rhs.m_element_count)
{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing move construction and assignment.

@hkaiser
Copy link
Member

hkaiser commented Apr 6, 2015

Is there a way to tell if an Archive type is an archive, and whether it's an input or output one? Or some other way to implement serialization support without ever including a single bit of the serialization library itself?

I don't see any, if I understand you correctly.

Would it make sense to introduce special traits for this? @K-ballo: I'm not sure what you have in mind, but wouldn't traits handle your use case?

@K-ballo
Copy link
Member

K-ballo commented Apr 6, 2015

Would it make sense to introduce special traits for this? @K-ballo: I'm not sure what you have in mind, but wouldn't traits handle your use case?

Traits would still require including some library code in order to support serialization. With Boost.Serialization, it is sometimes possible to implement serialize SFINAEd on Archive::is_saving, and thus does not require including anything extra. The existing of the is_saving member is interpreted as Archive modeling the Archive concept, and the save/load split can be done based on its value. This was likely just by chance for Boost.Serialization, but I would really like to see some similar way of adding serialization support without actually introducing a hard dependency nor a single extra #include.

@AntonBikineev AntonBikineev self-assigned this Apr 8, 2015
@sithhell
Copy link
Member

Looks good to me! I think we should merge the changes as soon as possible now.

@biddisco
Copy link
Contributor

Merging this will help with development of parcelport code which depends heavily on serialization.

hkaiser added a commit that referenced this pull request Apr 14, 2015
@hkaiser hkaiser merged commit 66ec7ef into master Apr 14, 2015
@sithhell sithhell deleted the hpx_serialization branch April 16, 2015 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants