Re:[boost] Re: Serialization Formal Review #2
Message: 11 Andreas Huber wrote:
1. On compilers supporting ADL, it seems the user-supplied serialization function for non-intrusive serialization
template<class Archive> void serialize(Archive & ar, gps_position & g, const unsigned int version);
could also be put into the namespace of the type that it serializes. If this is correct it should be mentioned in the tutorial & reference. If not, there should be a rationale why not.
I did try this in my environment VC 7.1 and it failed to work. Does anyone want to assert that it should or shouldn't work?
2. save/load naming: All functions that have to do with reading from an archive are named load*, all functions that have to do with writing to an archive are named save*. To me (non-native speaker) these names are generally associated with persistence only. Since the library is more general (one could use it for network communication, etc.) load/save are a bit missleading. I'd rather see read/write or other names that more clearly communicate what the functions are doing.
I used save/load consistently specifically to distinguish from read/write which seemed to me too suggestive of persistence, files and streams.
3. archive naming: The word archive also seems to be associated with persistence. I think serializer would much better describe the functionality. Another option I could live with is serialization_stream (I slightly disagree with the assertion in archives.html that an archive as modelled by this library is not a stream. It might not be what C++ folks traditionally understand a stream is but to me it definitely is a more general stream, i.e. a stream of objects/things/whatever).
Archive is perhaps a little too suggestive. I borrowed from MFC. Making changes would have a huge ripple effect through code, file names, namepace names and documentation. I don't find any proposals for alternate naming sufficiently compelling to justify this.
4. Object tracking 1: Seeming contradiction I've had difficulty determining how exactly object tracking works. I believe the associated docs are slightly contradictory:
- Under exceptions.html#pointer_conflict I read that the pointer_conflict exception is thrown when we save a class member through a pointer first and then "normally" (i.e. through a reference).
Correct.
- traits.html#tracking says "... That is[,] addresses of serialized objects are tracked if and only if an object of the same type is anywhere in the program serialized through a pointer."
Correct.
The second sentence seems to contradict the first one in that it does not specify that the sequence of saving is important
I see no conflict here
(I assume that pointer_conflict exception is not thrown if we save normally first and then through a pointer).
Correct
Moreover, I don't see how the library could possibly achieve the "anywhere in the program" bit.
It is tricky and it does work. I works by instantitiating a static variable. Static variables are constructed before main() is invoked. These static variables register themselves in global table when constructed. So even though a template is used "anywhere in the program" the entry in the global table exists before execution starts. I'm reluctant to include such information in the manual.
5. Object tracking 2: It should not be possible to serialize pointers referencing objects of types that have track_never tracking type.
LOL - tell that to library users. There are lots of things that users are convinced they have to do that I don't think are a good idea. As a library writer I'm not in position to enforce my ideas how users should use the library. On this particular point I would be very suspicious about such a usage but as practical matter I just have to include a warning and move on. special.html#objecttracking says: <quote> By definition, data types designated primitive by Implementation Level class serialization trait are never tracked. If it is desired to track a shared primitive object through a pointer (e.g. a long used as a reference count), It should be wrapped in a class/struct so that it is an identifiable type. The alternative of changing the implementation level of a long would affect all longs serialized in the whole program - probably not what one would intend. </quote>
I think serializing untracked pointers almost never makes sense. If it does anyway the pointed-to object can always be saved by dereferencing the pointer. Such an error should probably be signalled with an exception.
I don't understand this.
6. Object tracking 3: One should be able to activate tracking on the fly Rarely it makes sense to save even primitive types in tracked mode (e.g. the example quoted in 6.). That's why I think setting the tracking type per type is not flexible enough. It should be possible to override the default somehow. I could imagine to save such pointers as follows:
class T { ... int i; int * pI; // always points to i };
template<class Archive > void T::save(Archive &ar) const { ar << track( i ); // save the int here, keep track of the pointer ar << track( pI ); // only save a reference to the int }
Since it is an error to save an untracked type by pointer even the second line needs the track( ) call.
A main function of using a trait is to avoid instantiation of code that won't ever be used. If I understand your proposal we would have to instantiate tracking code even if it isn't used. But my main objection thing about this proposal is that it introduces a runtime modality into the type traits. This sound like it would very hard to debug and verify. I see lots of difficulties and very little advantage to trying to add this.
7. I think the enum tracking_type should rather be named tracking_mode as the _type suffix suggests something else than an enum.
I don't think the change is sufficiently compelling to justify the hassle associated with this.
8. I don't understand how function instantiation as described under special.html#instantiation works. The description seems to indicate that all possible archive classes need to be included in the files of all types that are exported. Is that true?
True. All archives that might be used have to be included. This is an artifact of using templates rather than virtual functions. Templates regenerate the code for each combination of serializable type and archive. This results in faster code - but more of it. There were strong objections raised in the first review about the fact it used virtual functions rather then templates - so it was changed to templates here.
9. The library registers all kinds of stuff with static instances. This precludes its use outside main().
Hmmm - that would be news to me. Robert Ramey
Robert Ramey wrote:
Message: 11 Andreas Huber wrote:
1. On compilers supporting ADL, it seems the user-supplied serialization function for non-intrusive serialization
template<class Archive> void serialize(Archive & ar, gps_position & g, const unsigned int version);
could also be put into the namespace of the type that it serializes. If this is correct it should be mentioned in the tutorial & reference. If not, there should be a rationale why not.
I did try this in my environment VC 7.1 and it failed to work. Does anyone want to assert that it should or shouldn't work?
I'm unsure about VC7.1. It is supposed to work with conformant compilers as detailed here: http://www.boost.org/libs/smart_ptr/intrusive_ptr.html#Introduction However, the situation is different there as the called function only accepts exactly one parameter. IIRC, if ADL works correctly it looks in all namespaces of all function arguments for a match. I'll see whether I find this rule in the standard.
2. save/load naming: All functions that have to do with reading from an archive are named load*, all functions that have to do with writing to an archive are named save*. To me (non-native speaker) these names are generally associated with persistence only. Since the library is more general (one could use it for network communication, etc.) load/save are a bit missleading. I'd rather see read/write or other names that more clearly communicate what the functions are doing.
I used save/load consistently specifically to distinguish from read/write which seemed to me too suggestive of persistence, files and streams.
Streams have nothing to do with persistence and files, do they? As I mentioned below an archive *is* in some way a stream, so read/write seems more appropriate, especially since standard stream classes also use this terminology. Save/load seems *much* closer to persistence and files than read/write is.
3. archive naming: The word archive also seems to be associated with persistence. I think serializer would much better describe the functionality. Another option I could live with is serialization_stream (I slightly disagree with the assertion in archives.html that an archive as modelled by this library is not a stream. It might not be what C++ folks traditionally understand a stream is but to me it definitely is a more general stream, i.e. a stream of objects/things/whatever).
Archive is perhaps a little too suggestive. I borrowed from MFC. Making changes would have a huge ripple effect through code, file names, namepace names and documentation. I don't find any proposals for alternate naming sufficiently compelling to justify this.
Global search/replace is your friend ;-). Seriously, if I'm the only one with these concerns I rest my case now and will happily use your library as is.
4. Object tracking 1: Seeming contradiction I've had difficulty determining how exactly object tracking works. I believe the associated docs are slightly contradictory:
- Under exceptions.html#pointer_conflict I read that the pointer_conflict exception is thrown when we save a class member through a pointer first and then "normally" (i.e. through a reference).
Correct.
- traits.html#tracking says "... That is[,] addresses of serialized objects are tracked if and only if an object of the same type is anywhere in the program serialized through a pointer."
Correct.
The second sentence seems to contradict the first one in that it does not specify that the sequence of saving is important
I see no conflict here
Yeah, this was a misunderstanding on my part. Sorry for the noise!
(I assume that pointer_conflict exception is not thrown if we save normally first and then through a pointer).
Correct
Moreover, I don't see how the library could possibly achieve the "anywhere in the program" bit.
It is tricky and it does work. I works by instantitiating a static variable. Static variables are constructed before main() is invoked. These static variables register themselves in global table when constructed. So even though a template is used "anywhere in the program" the entry in the global table exists before execution starts. I'm reluctant to include such information in the manual.
I agree that such information should probably not be in the tutorial but it could certainly be in the reference.
5. Object tracking 2: It should not be possible to serialize pointers referencing objects of types that have track_never tracking type.
LOL - tell that to library users. There are lots of things that users are convinced they have to do that I don't think are a good idea. As a library writer I'm not in position to enforce my ideas how users should use the library. On this particular point I would be very suspicious about such a usage but as practical matter I just have to include a warning and move on.
special.html#objecttracking says:
<quote> By definition, data types designated primitive by Implementation Level class serialization trait are never tracked. If it is desired to track a shared primitive object through a pointer (e.g. a long used as a reference count), It should be wrapped in a class/struct so that it is an identifiable type. The alternative of changing the implementation level of a long would affect all longs serialized in the whole program - probably not what one would intend. </quote>
I think serializing untracked pointers almost never makes sense. If it does anyway the pointed-to object can always be saved by dereferencing the pointer. Such an error should probably be signalled with an exception.
I don't understand this.
My point is that if a user absolutely needs to save something pointed to by an untracked pointer then she can always do so by dereferencing the pointer first (I highly doubt that this is necessary very often). int i = 5; int * pI = &i; ar & *pI; // This works ar & pI; // This doesn't (exception is thrown) It makes the user much more aware what really happens if he saves something pointed to by an untracked pointer. This way it is also absolutely clear what the users responsibility is when such a pointer is *loaded*. Since dereferencing a pointer that has an undefined value is, um, undefined behavior the user must set the pointer *before* dereferencing it and loading the value from the archive.
6. Object tracking 3: One should be able to activate tracking on the fly Rarely it makes sense to save even primitive types in tracked mode (e.g. the example quoted in 6.). That's why I think setting the tracking type per type is not flexible enough. It should be possible to override the default somehow. I could imagine to save such pointers as follows:
class T { ... int i; int * pI; // always points to i };
template<class Archive > void T::save(Archive &ar) const { ar << track( i ); // save the int here, keep track of the pointer ar << track( pI ); // only save a reference to the int }
Since it is an error to save an untracked type by pointer even the second line needs the track( ) call.
A main function of using a trait is to avoid instantiation of code that won't ever be used. If I understand your proposal we would have to instantiate tracking code even if it isn't used.
Couldn't track() instantiate the code?
But my main objection thing about this proposal is that it introduces a runtime modality into the type traits.
I agree that it's a bit of a hack but the current way of doing things (wrapping all untracked types into new types) doesn't sound very appealing either. It forces me to change my design. Only rarely but I still have to change it. track() would allow me to save anything *as is*.
This sound like it would very hard to debug and verify. I see lots of difficulties and very little advantage to trying to add this.
Since I haven't had the time to study the implementation I can only guess & assume but I don't see how this is hard to debug and verify. As mentioned under 5. above, it should be an error to save through an untracked pointer. I would therefore be forced to *always* use track() whenever I want to save an untracked pointer directly. This makes it immediately visible in the user code. The (IMHO big) advantage is that I don't have to pull tricks to save something through an untracked pointer.
7. I think the enum tracking_type should rather be named tracking_mode as the _type suffix suggests something else than an enum.
I don't think the change is sufficiently compelling to justify the hassle associated with this.
Again: Global search and replace is your friend ;-). Seriously, if nobody else has concerns here I'm happy with it as is.
8. I don't understand how function instantiation as described under special.html#instantiation works. The description seems to indicate that all possible archive classes need to be included in the files of all types that are exported. Is that true?
True. All archives that might be used have to be included. This is an artifact of using templates rather than virtual functions. Templates regenerate the code for each combination of serializable type and archive. This results in faster code - but more of it. There were strong objections raised in the first review about the fact it used virtual functions rather then templates - so it was changed to templates here.
Ok, but IIUC this undermines the original promise that a class implementing serialize or safe/load can be serialized to *any* archive, even future ones without changing its implementation. I see that I wouldn't need to change the serialization *code* but I'd need to add an include to the .cpp file, right?
9. The library registers all kinds of stuff with static instances. This precludes its use outside main().
Hmmm - that would be news to me.
Maybe I haven't stated it clearly enough: Your library cannot be used to save/load anything before main() is entered nor can it be used to save/load anything after main() has been left. This stems from the fact that code running before or after main() is ultimately always called from a constructor or destructor of some static instance (threads aside for the moment). Since the C++ standard does not guarantee anything about the sequence of construction of these instances, code saving/loading stuff could well be called *before* all the registering is done. OTOH, when I save something from a destructor of a static instance, this could be done after you have destructed a table containing registration information. If I'm reading the standard correclty, then it does not currently allow us to work around these issues *in any way*. Therefore, this limitation should be mentioned in the docs. Regards, Andreas
----- Original Message ----- From: "Andreas Huber" <ah2003@gmx.net> To: <boost@lists.boost.org> Sent: Monday, April 26, 2004 11:40 AM Subject: [boost] Re: Serialization Formal Review #2
Robert Ramey wrote:
Message: 11 Andreas Huber wrote:
1. On compilers supporting ADL, it seems the user-supplied serialization function for non-intrusive serialization
template<class Archive> void serialize(Archive & ar, gps_position & g, const unsigned int version);
could also be put into the namespace of the type that it serializes. If this is correct it should be mentioned in the tutorial & reference. If not, there should be a rationale why not.
I did try this in my environment VC 7.1 and it failed to work. Does anyone want to assert that it should or shouldn't work?
I'm unsure about VC7.1. It is supposed to work with conformant compilers as detailed here:
http://www.boost.org/libs/smart_ptr/intrusive_ptr.html#Introduction
However, the situation is different there as the called function only accepts exactly one parameter.
It doesn't matter. ADL should find the overload, provided that 'serialize' is not qualified at the point of the call. In my own library I use very similar read/write overloads that are, indeed, placed in the namespace of the corresponding class and found by ADL. Yes, it works with VC 7.1.
"Peter Dimov" <pdimov@mmltd.net> writes:
http://www.boost.org/libs/smart_ptr/intrusive_ptr.html#Introduction
However, the situation is different there as the called function only accepts exactly one parameter.
It doesn't matter. ADL should find the overload, provided that 'serialize' is not qualified at the point of the call.
In my own library I use very similar read/write overloads that are, indeed, placed in the namespace of the corresponding class and found by ADL. Yes, it works with VC 7.1.
The recommended "overload in the library namespace" procedure was a big concern for me, but I didn't post anything because I couldn't remember why it's an issue. I have in my mind, left over from my work with Peter on this problem, a rule that in the current language you shouldn't do that, and should rely instead on ADL. I don't know why I think that anymore, but I *can* say that Peter should be listened to in this area; he's more aware of the issues than almost anyone. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
"Andreas Huber" <ah2003@gmx.net> wrote in message news:loom.20040426T101217-326@post.gmane.org...
Robert Ramey wrote:
Message: 11 Andreas Huber wrote:
[snip]
I used save/load consistently specifically to distinguish from read/write which seemed to me too suggestive of persistence, files and streams.
Streams have nothing to do with persistence and files, do they? As I mentioned below an archive *is* in some way a stream, so read/write seems more appropriate, especially since standard stream classes also use this terminology. Save/load seems *much* closer to persistence and files than read/write is.
3. archive naming: The word archive also seems to be associated with persistence. I think serializer would much better describe the functionality. Another option I could live with is serialization_stream (I slightly disagree with the assertion in archives.html that an archive as modelled by this library is not a stream. It might not be what C++ folks traditionally understand a stream is but to me it definitely is a more general stream, i.e. a stream of objects/things/whatever).
Archive is perhaps a little too suggestive. I borrowed from MFC. Making changes would have a huge ripple effect through code, file names, namepace names and documentation. I don't find any proposals for alternate naming sufficiently compelling to justify this.
Global search/replace is your friend ;-). Seriously, if I'm the only one with these concerns I rest my case now and will happily use your library as is.
From Merriam Webster Dictionary:
Main Entry: 1ar·chive Pronunciation: 'är-"kIv Function: noun Etymology: French & Latin; French, from Latin archivum, from Greek archeion government house (in plural, official documents), from archE rule, government -- more at ARCH- : a place in which public records or historical documents are preserved; also : the material preserved -- often used in plural and Main Entry: 2archive Function: transitive verb Inflected Form(s): ar·chived; ar·chiv·ing : to file or collect (as records or documents) in or as if in an archive IMO Archive is about as good a name as we'll get. Jeff F
[snip]
From Merriam Webster Dictionary:
Main Entry: 1ar·chive Pronunciation: 'är-"kIv Function: noun Etymology: French & Latin; French, from Latin archivum, from Greek archeion government house (in plural, official documents), from archE rule, government -- more at ARCH- : a place in which public records or historical documents are preserved; also : the material preserved -- often used in plural
and
Main Entry: 2archive Function: transitive verb Inflected Form(s): ar·chived; ar·chiv·ing : to file or collect (as records or documents) in or as if in an archive
These entries also seem to put archive pretty close to persistence. My point was that serialization is not only about persistence. Persistence is surely a big use case but not the only one.
IMO Archive is about as good a name as we'll get.
Even if one uses serialization in conjunction with e.g. network communication? "archive" doesn't sound right to me in this environment. IMHO, "serializer" would be much better because it does not make any suggestions where the data is going to end up. However, since I seem to be the only one with these concerns I won't push this any further. Regards, Andreas
Andreas Huber wrote: [snip]
Even if one uses serialization in conjunction with e.g. network communication? "archive" doesn't sound right to me in this environment.
Good point. John Nagel's post at: http://groups.google.com/groups?q=g:thl3471000725d&dq=&hl=en&lr=&ie=UTF-8&oe=UTF-8&scoring=d&selm=e7Thc.54263%24VK6.45375%40newssvr25.news.prodigy.com suggests he might be interested in such a "marshalling" application of this serializer library.
IMHO, "serializer" would be much better because it does not make any suggestions where the data is going to end up. However, since I seem to be the only one with these concerns I won't push this any further.
Although I haven't looked closely at serialization, I think Andreas makes a good point. In addition, it was not at first apparent to me that although: struct A{int i, char c}; is laid out in memory with c following i, the serializer could just as well store it as i following c. The secret being that only one templated serialize function is used to both store and load, with the store or load operation selected by the Archive template parameter. I admit that if I'd read last paragraph under the Free Function section, I would have realized that. This raises another minor point. At first, when I saw: ar & degrees; in member function: void gps_position::serialize(Archive & ar, const unsigned int version) I jumped to the conclusion that this was a declaration. Maybe something like: ar &= degrees; would make the statement's intention less mistakable.
Larry Evans wrote: [snip]
This raises another minor point. At first, when I saw:
ar & degrees;
in member function:
void gps_position::serialize(Archive & ar, const unsigned int version)
I jumped to the conclusion that this was a declaration. [snip] OOPS. If it were, it would have had a = after degrees. Maybe it's not that important after all :)
participants (6)
-
Andreas Huber -
David Abrahams -
Jeff Flinn -
Larry Evans -
Peter Dimov -
Robert Ramey