[boost] Re: Serialization Library Review

From: Fredrik Blomqvist (fredrik_blomqvist_at_[hidden])
Date: 2002-11-23 12:25:01

Hi,

I've been following the library and the discussion surrounding it since it
was first announced on the list.
I believe it is a very important and much needed component and I thank
Robert for bringing it forward to the boost community!
For the review I have gathered some issues I would like to bring to
attention. (presented in no particular order)

Features / Implementation:

* I haven't seen any reaction on the friend-declaration change needed in
shared_count.hpp to support serialization of shared_ptrs. Would this
coupling shared_ptr<->serializer be ok for an official boost release?
Anyhow, I'd like to point out an alternative implementation approach to
handle non-intrusive smart-ptrs (like shared_ptr) in a non-intrusive way ;-)
(I've briefly mentioned this approach in private communication with Robert)
-----
The method relies on a map of raw_ptr->shared_ptr that is maintained at
deserialization time.
 At load-time the initially empty map is populated with the shared_ptrs that
show up as destination
targets. The code first looks in the map to see if a previous shared_ptr
exists been handled, if so it is initialized by copying from the one in the
map. Only if it's the very first ptr to a new object or the first shared_ptr
a shared_ptr::reset(raw_ptr) is performed, followed by insertion in the map.
This will maintain correct ref-counts for smart-ptrs without knowledge of
internal functionality.
  Note that this approach _only_ saves the raw-ptr and nothing else relating
to the shared_ptr to the archive and that it even manages to implicitly
repair faulty shared_ptr links :) . Finally weak_ptrs are easily handled
using the same shared_ptr base.
  I have used this implementation in a modified CommonC++ serializer and I
believe it would work as a specialization for shared_ptr<T> in Roberts
serializer if needed. Altough I'm not 100% sure how much the lack of a
baseclass would complicated matters.
(hmm, hope the above makes alteast some sense.. ;)
-----

* I'd like to see a boost::serializer handle all/most of the std/STL and
boost components "out-of-the-box".
Even though the current implementation shows how to handle both auto_ptr and
shared_ptr in the examples they're IMO too important not to be included in
the official core-package. Both from a QoI point of view but also purely
convenience.
Structures that come to mind are:
   std::auto_ptr
   std::stack
   std::queue/priority_queue
   boost::scoped_ptr
   boost::shared/weak_ptr
   boost::tuple
   boost::array
   boost::multi_array
   etc ...

* The handling of the different (like the STL) components should be broken
into separate .hpp files.
(altough possibly retaining a convenience wrapper)

* Since the library already supports hash_map and slist etc I guess adding
support for the popular rope string-container would be a simple addition
also.

* Regarding support for serialization of the container adaptors like stack
and queue I propose a solution something like this to be able to take
advantage of possible optimized serialization of the basecontainer. This
example serializes a std::stack at the expense of an extra copying step
which I unfortunately see no conforming way to avoid (since the adaptor
templates are defined to hide the container). Is even this standard
conforming btw? can you assume that the inner-container is always named 'c'?
----------------------------------------------------------------------------
---------
template <typename T, class C>
struct extract_container_t : std::stack<T,C> {
    typedef std::stack<T,C> stack_type;
    extract_container_t() {}
    explicit extract_container_t(const stack_type& s) : stack_type(s) {}
    const stack_type::container_type& get() const { return c; }
    stack_type::container_type& getref()               { return c; }
};

template<class T, class C>
inline void save_template(boost::basic_oarchive & ar, const std::stack<T,C>
&t, long) {
    ar << extract_container_t<T,C>(t).get();
}

template<class T, class C>
inline void load_template(boost::basic_iarchive & ar, std::stack<T,C> &t,
boost::version_type file_version, long) {
    extract_container_t<T,C> tmp;
    ar >> tmp.getref();
    t = std::stack<T,C>(tmp.get());
}
----------------------------------------------------------------------------
---------

* If possible, I'd like to be able to build a version of the library with
disabled exception-handling.
(as several other boost libs already provide)

* If it's possible to simplify the library and/or refactor it into separate
components I'd like to see that.

* nitpick: code cotains some superflous inlines in a couple of places.

Design:

*The core design is the best I've seen of a serializer. The key features:
Non-intrusive serialization (no baseclass!), versioning, standard RTTI and
good STL support. This coupled with being even easier to use in day-to-day
code than for example MFC makes it very attractive in my eyes!

* Q: Regarding serializable structures. How should other boost components
handle interaction with a serializer? Should it be a task for the
serialization-lib or should each component/structure be allowed to do as it
see fit?
In short; should we propose some convention or policy to be used in boost
for serializable structures up front?

Documentation / Examples:
* Perhaps provide a more explicit migration and benefits page for users
previously used to
  the MFC or CommonC++ style serializers and similar.

* Should remove the comments talking about ref-counts in the auto_ptr
example

* The shared_ptr example should IMO be extended to also use weak_ptr but
most of all serialize pointer structures really demanding ref-counting (ie
that would otherwise fail)

* Should supply atleast one example that illustrates a "different" handling
of to the archive like compression or network transport (those questions
could be added to a FAQ if nothing else)

Platform specific:
* The library worked correctly with the tests I did on a clean VC7 (I had
some problems with VC6 on a previous version of the lib but I didn't have
time to try VC6 again. IIRC one problem was the lack of string::data() in
VC6 stl).

*As a VC user I would certainly benefit from a workaround for the 'baseclass
withouth virtual functions' problem mentioned in the quirks.

My context:
*I first came in contact with serialization a couple of years ago using the
MFC serializer. Later I used and took part in developement of an in-house
serializer of similar design. For the past year I have personally used a
modified CommonC++  serializer, to which I added better STL and boost
support, including non-intrusive shared_ & weak_ptr handling (as detailed
above). I have also used
The showstoppers of the above libraries in the "boost-age", and what made me
modify the CommonC++ impl. and now anticipate Roberts serializer, was the
usage of specific macro based RTTI code, bad STL integration and no supprt
for smart ptrs. Basically exactly the points initially identified by Robert
also :)

Conclusion:
*I'm in favor of the library and would very much like to see a component
like this in boost. For my personal needs (ie boost-grade replacement for
CommonC++) the library would be more than adequate. Myself I have more or
less always considered serialization as a specific binary format and I have
thus not been that concerned with platform independence, XML and such.
(Altough I certainly wouldn't mind having such support too!).
 If I understand the more recent posts regarding the lib, the consensus
right now seems to be that there are too many loose ends regarding different
peoples needs in a library like this that an inclusion isn't possible
without modifications/additions. (?)
I would vote Yes if the library can be reasonably shown to be able to evolve
in a graceful way to accomodate the likely future changes additions.

Regards-in-a-hurry
/Fredrik Blomqvist

(I appologise for my unnecessary late comments..)