From: Matthias Troyer (troyer_at_[hidden])
Date: 2004-04-24 15:34:44
On Apr 12, 2004, at 10:09 PM, Jeff Garland wrote:
> 2) I am particularly interested in hearing from parties that have
> tried to
> extend the library by adding serialization for a new type or by
> building a new
> type of archive. The ability to easily extend the library for new
> types and
> novel types of archives is critical for this library.
I have tried extending the library since I want to swap our old
serialization library used at http://alps.comp-phys.org with a Boost
serialization library as soon as possible. It was important to keep the
same archive format, since we have gigabytes of data collected over ten
years in millions of CPU-hours and need to be able to read those with
both new and old codes. With the help of Robert I managed to convince
myself that this is indeed possible. The only issue I have is that the
documentation needs to be improved.
> 3) If you reviewed the library during the first review, I would like
> your
> evaluation of whether the new version of the library addresses your
> original
> concerns. Also, if this is a re-review I would appreciate if you
> could send
> me a weblink from the mail archive to your original review posting(s).
In the first round I voted against inclusion in Boost because i) the
design was not flexible enough to allow reading my old legacy formats
and ii) the design inhibted some optimizations I need for large data
sets. Robert did an excellent job in addressing this and other issues.
The new version is greatly improved and I am satisfied with the design.
> As usual, please state in review comments how you reviewed the library
> and
> whether the you think the library should be accepted into Boost.
> Further
> guidelines for writing reviews can be found on the website at:
>
> http://www.boost.org/more/formal_review_process.htm#Comments
> What is your evaluation of the design?
The current design offers the flexibility needed for all serialization
tasks I can think of and makes it possible to implement serialization
without any performance penalty.
> What is your evaluation of the implementation?
I did not look at many details of the implementation but have a few
comments:
1. there are still a few "dangerous" code portions, such as
load_binary(const_cast<char *>(s.data()), l);
in line 89 of file basic_binary_iprimitive.ipp . It is stated in a
comment that this might be unsafe, but it is probably OK on all current
implementations of the standard.
2. the comments in the source files are at several places out of sync
with the implementation, they sometimes still refer to old designed of
the library. A thorough check of all files is needed.
3. looking at the implementation after reading the documentation is
confusing at some places because of tricks used to work around compiler
deficiencies. For example, the documentation discusses load() and
save() functions but the implementations use load_override() and
save_override() functions. I would prefer to have the implementation be
consistent with the documentation and the workarounds included only in
#if .... #endif guards.
> What is your evaluation of the documentation?
The documentation is OK for using the library, but more and better
documentation is needed for:
1. implementing new archive types: there, as Robert is aware, the
example in the documentation is incorrect. A more extended
documentation would be very useful.
2. Better documentation for the classes used to track classes, class
version, objects, etc.. For example, in newarchive.html there is a
table listing some types (class_id_optional_type is listed twice???)
but I am missing
a) more information about the purpose of these types
b) I found some additional types such as class_name_type, which is
used in the implementation of some archives but not documented.
3. Documentation for the workarounds used to placate deficient
compilers (e.g. save_override, ...) is missing
4. Documentation of archive formats, especially what class and object
information is stored. I want to be able to predict, from reading the
documentation, what exactly will be written to the archive and in which
order. This is essential when exchanging data with an application that
does not use this library.
> What is your evaluation of the potential usefulness of the library?
In my opinion this will be among the most useful and most widely Boost
libraries.
> Did you try to use the library? With what compiler? Did you have
> any problems?
I tried using the library on MacOS X 10.3 using Apple's version of the
g++ 3.3 compiler. I encountered two problems.
1. archive /wcslen.hpp does not compile because for some unknown reason
boost/config.hpp defines BOOST_NO_CWCHAR, although the <cwchar> header
exists. Is there a deep reason for this configuration option on MacOS X
or is this a bug in the configuration?
2. trying to compile text_wiarchive.cpp and xml_wiarchive.cpp I get
error messages such as
boost/archive/iterators/remove_whitespace.hpp:146: error: `
boost::archive::iterators::filter_iterator<Predicate,
Base>::m_predicate'
has incomplete type
I did not investigate this further at this time because of lack of
time. and because I do not currently need the wide stream archive
types.
> How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
Being busy and traveling from Switzerland to California has not allowed
me time for an in-depth study of all aspects. I focused compiling the
library under MacOS X, writing short example programs to studdy the
archive formats with, and implementing a new archive type.
> Are you knowledgeable about the problem domain?
I have use serialization in a portable binary format on a daily basis
for a decade.
To summarize, I vote for accepting the library into Boost but would
like to see the documentation improved.
Matthias