[boost] Re: Serialization Library Review

[Alberto Barbati]

Hi David,

- What is your evaluation of the design?

I like the design very much. It's neat and far superior to other alternatives I evaluated in the same problem domain. I have just a few observations:

1) I don't like the non-intrusive way of specifying the version/save/load operation. The rationale for having the serialization<> template in the global namespace is quite weak. The alternative of using free functions is much better IMO. The library should use unqualified calls to those free functions and Koenig look-up would do the rest. This solution has also the benefit that specialization is replaced by overloading, a feature that is more easily supported by compilers. In this framework the "boost::serialization_detail" hack (which, in fact, is no more than a compiler hack, the natural syntax being correct to me) would probably not be needed.

2) A most needed addition to the design is to provide a sort of "registry" object. This object would present one single public method with the same semantic as an archive's register_type(). Then the archives constructors would take such a "registry" object and immediately invoke register_type() for each type contained there. With this approach, we have a better partition of responsibilties. The module that actually serialize objects no longer depends on the list of all serializable objects in the program, it needs only be given a registry object instance. Typically, such instance would be a singleton. How the registry is created and initialized is application responsibility: there may be a know-it-all module or the programmer may use macros a la MFC to let each class self-register.

- What is your evaluation of the implementation?

I did not go very deep in the implementation. However, I have again a few comments:

1) exception handling should be made optional. This is as easy as replacing each "throw" with a call to boost::throw_exception and use macro BOOST_NO_EXCEPTIONS to #ifdef the try/catch lines in function serialization<T>::load_ptr (file serialization_imp.hpp).

2) in the text_oarchive and boarchive constructors, there is a seekp(0) on the given stream. This is an error, IMO, and the offending lines should be removed. First of all there is no reason why the constructor should move the position in the stream: the programmer may have have all reasons not to like that (for example the serialization data could be embbedded in a larger file). Second, and most important, it is not ANSI conforming. In fact the standard says explicitly "If sp has not been obtained by a previous successful call to one of the positioning functions on the same stream the effects are undefined." There is at least one platform (Metrowerks CodeWarrior, MSL) where seekp(0) fails on newly-created ostringstreams, leaving the stream in "failed" state. As, in the current implementation, there is no status check after the seekp(0), any further operation on the given stream will fail as well.

3) the implementation should not assume that slist and hashed containers are in the std namespace. The macro BOOST_STD_EXTENSION_NAMESPACE should be used instead. This is required on at least one platform (CodeWarrior).

4) at least in all public headers, unused function parameters should be either unnamed or their names commented out, in order to avoid compilation warnings from pedantic compilers.

- What is your evaluation of the documentation?

The documentation is quite basic, but is a good start. The tutorial code should is probably outdated and should be reviewed (for example all friend declarations are missing the "class" keyword). In several places there are problems with < and > being replaced with HTML-ish &lt; and &gt;.

One note: the library, as it is, *does not* support Unicode output, as stated. The library supports wide streams, yes, but that does not mean Unicode support. In fact, the standard says explictly that the conversion facet codecvt<wchar_t,char,mbstate_t> used by default by wide streams shall "implement a degenerate conversion". For example, if wchar_t has 16 bits and char has 8 bits (typical situation of x86-based plaftforms), that means that the high byte of each wchar_t is simply discarded. Thus Unicode characters outside the 0-255 range will not be output correctly, unless an alternative conversion facet is imbued in the stream before passing it to the library. (Shameless plug: that's why I've tried to propose alternate codecvt facets for Unicode... but no one seems to realize the real issue.)

- What is your evaluation of the potential usefulness of the library?

The potential usefulness of library is huge. However the lack of a centralized registration component for polymorphic types restricts its applicatibility.

- Did you try to use the library? With what compiler? Did you have any problems?

I tried little demo programs with two different compilers. That was not a deep test with live data, although I plan to do it in the next few days. I tested both polymorphic and non-polymorphic types, but no templates.

On VC++ 7.0 (.NET) it all went good at the first try. However the restriction, cited in the documentation, of having to add /OPT:NOREF to the linker options is too hard to swallow. In a typical setting, this can increase the executable size by up to 100% or even more, as such is the typical size of unreferenced data. This problem will have to be addressed explicitly.

On Metrowerks CodeWarrior I had a few problems. A few of them I already described above. Another one come from the fact that the library uses (correctly) the trait is_base_and_derived in the implementation of template class base_object. is_base_and_derived depends on is_convertible that is broken on CodeWarrior. I had to comment out the static assert line to let the program compile.

- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

I read the documentation quickly, then write those test programs. I traced the execution in the debugger to discover the seekp(0) issue and to have glance at the library internals, but that does not account for in-depth study.

- Are you knowledgeable about the problem domain?

I can say that I am a bit knowledgeable. I worked on (and know all quirks of) the MFC implementation and tried myself at least two different approaches, although they were limited in the scope of the respective applications and not general-purpose components.

- Do you think the library should be accepted as a Boost library?

My opinion is that the library should not be accepted as it is, but has huge potential. There is indeed little space for improvements, but a few features, such as the registration of polymorphic types, should defintely be addressed before prime time.

Alberto Barbati



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost