[serialization] Header Ordering Fixes
Robert, I'm just about done. I'm unable to test on cw-8.3 at the moment because the test library seems to be broken there. As for cw-9.4, the export tests never worked there, presumably for the same reason I'm seeing: when a class template static member is used only as the result of another template instantiation, it may never get initialized. As a result, base/derived relationships are not getting registered. Aside from that, all export tests seem to be passing, including release mode tests on vc-8.0. I was able to clean up a *lot* of messy code. Would you like me to: 0. Check my code in on the trunk 1. Check my code in on a branch so you can review it 2. Post patches (not recommended; the diffs are large) 3. Something else? -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Robert,
I'm just about done. I'm unable to test on cw-8.3 at the moment because the test library seems to be broken there. As for cw-9.4, the export tests never worked there, presumably for the same reason I'm seeing: when a class template static member is used only as the result of another template instantiation, it may never get initialized. As a result, base/derived relationships are not getting registered.
That's always been my guess - but I never found a way to fix it. I didn't have CW on my own machine so I wasn't really able to experiment enough to look for a solution - if there is one.
Aside from that, all export tests seem to be passing, including release mode tests on vc-8.0.
Note that I had a lot of problems on many platforms with code not being instantiated in release mode. I recommend testing on all platforms with release mode
I was able to clean up a *lot* of messy code.
Would you like me to:
0. Check my code in on the trunk
Without having looked at anything, I'm assuming that export.hpp has been pretty much replaced and I would be fine with seeing it just checked into the trunk. I wasn't aware that there was a *lot* of messy code (except perhaps workarounds for compiler quirks). Maybe we have a different criteria for "messy". If this confined to a few modules, maybe the most practical would be to email them to me so I might have a look at them. If its a lot of modules, I would like to consider it some more. Note that the trunk is undergoing some flux as the adjustments to array serialization are being integrated - this might affect the course of action here. Robert Ramey
1. Check my code in on a branch so you can review it 2. Post patches (not recommended; the diffs are large) 3. Something else?
Robert Ramey wrote:
David Abrahams wrote:
Robert,
I'm just about done. I'm unable to test on cw-8.3 at the moment because the test library seems to be broken there. As for cw-9.4, the export tests never worked there, presumably for the same reason I'm seeing: when a class template static member is used only as the result of another template instantiation, it may never get initialized. As a result, base/derived relationships are not getting registered.
That's always been my guess - but I never found a way to fix it. I didn't have CW on my own machine so I wasn't really able to experiment enough to look for a solution - if there is one.
I never found a solution, and I tried and tried :-( And eventually gave up. Although having a small repeatable test case would help. The last time I looked at this problem in the serialization lib the smallest test case involved a very large preprocessed result file. -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim - grafikrobot/yahoo
Rene Rivera <grafik.list@redshift-software.com> writes:
I never found a solution, and I tried and tried :-( And eventually gave up. Although having a small repeatable test case would help. The last time I looked at this problem in the serialization lib the smallest test case involved a very large preprocessed result file.
-- Dave Abrahams Boost Consulting www.boost-consulting.com
"Robert Ramey" <ramey@rrsd.com> writes:
David Abrahams wrote:
Robert,
I'm just about done. I'm unable to test on cw-8.3 at the moment because the test library seems to be broken there. As for cw-9.4, the export tests never worked there, presumably for the same reason I'm seeing: when a class template static member is used only as the result of another template instantiation, it may never get initialized. As a result, base/derived relationships are not getting registered.
That's always been my guess - but I never found a way to fix it. I didn't have CW on my own machine so I wasn't really able to experiment enough to look for a solution - if there is one.
Aside from that, all export tests seem to be passing, including release mode tests on vc-8.0.
Note that I had a lot of problems on many platforms with code not being instantiated in release mode. I recommend testing on all platforms with release mode
Yeah, it looks like vc-7.0 is still eliminating some of the startup code. My biggest problem is that I don't understand what you're registering and where, and I can't understand the error messages I see. So for example, when I get an "unregistered class" exception, what does that mean? It isn't at all obvious what constitutes registration of a class.
I was able to clean up a *lot* of messy code.
Would you like me to:
0. Check my code in on the trunk
Without having looked at anything, I'm assuming that export.hpp has been pretty much replaced and I would be fine with seeing it just checked into the trunk.
I wasn't aware that there was a *lot* of messy code (except perhaps workarounds for compiler quirks).
Yes, a great deal of the mess was workarounds (sometimes workarounds that were not effective), but lots was just functionality implemented in ways that were needlessly convoluted.
Maybe we have a different criteria for "messy". If this confined to a few modules, maybe the most practical would be to email them to me so I might have a look at them. If its a lot of modules, I would like to consider it some more. Note that the trunk is undergoing some flux as the adjustments to array serialization are being integrated - this might affect the course of action here.
The large changes are confined to a few modules. There are lots of smaller changes. Just tell me what you want me to do. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Yeah, it looks like vc-7.0 is still eliminating some of the startup code. My biggest problem is that I don't understand what you're registering and where, and I can't understand the error messages I see. So for example, when I get an "unregistered class" exception, what does that mean? It isn't at all obvious what constitutes registration of a class.
Basically it means that it an attempt to retrieve an instance from the global table of extended_type_info entries was not successful. The main function of export.hpp is to be sure that all that there is an extended_type_info entry for each exported type. This could fail due to compiler bug, bug in export.hpp or the user might have failed to use BOOST_EXPORT for that type.
I was able to clean up a *lot* of messy code.
Would you like me to:
0. Check my code in on the trunk
Without having looked at anything, I'm assuming that export.hpp has been pretty much replaced and I would be fine with seeing it just checked into the trunk.
I wasn't aware that there was a *lot* of messy code (except perhaps workarounds for compiler quirks).
Yes, a great deal of the mess was workarounds (sometimes workarounds that were not effective), but lots was just functionality implemented in ways that were needlessly convoluted.
Maybe we have a different criteria for "messy". If this confined to a few modules, maybe the most practical would be to email them to me so I might have a look at them. If its a lot of modules, I would like to consider it some more. Note that the trunk is undergoing some flux as the adjustments to array serialization are being integrated - this might affect the course of action here.
The large changes are confined to a few modules. There are lots of smaller changes.
Just tell me what you want me to do.
might as well just check 'em in to the trunk Robert Ramey
"Robert Ramey" <ramey@rrsd.com> writes:
David Abrahams wrote:
Yeah, it looks like vc-7.0 is still eliminating some of the startup code. My biggest problem is that I don't understand what you're registering and where, and I can't understand the error messages I see. So for example, when I get an "unregistered class" exception, what does that mean? It isn't at all obvious what constitutes registration of a class.
Basically it means that it an attempt to retrieve an instance from the global table of extended_type_info entries was not successful. The main function of export.hpp is to be sure that all that there is an extended_type_info entry for each exported type. This could fail due to compiler bug, bug in export.hpp or the user might have failed to use BOOST_EXPORT for that type.
Yes, I understand it in abstract terms. What I can't tell, when I get one of those exceptions, is what I have to make happen. What function should have been called (or object constructed), and with what arguments? This gives me a hint, for a different exception, that void_cast_register should have been called (although it still leaves out lots of info, like the identities of the base and derived classes). unregistered_cast // base - derived relationship not registered with // void_cast_register I really need the same kind of information about unregistered_class. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams <dave@boost-consulting.com> writes:
I really need the same kind of information about unregistered_class.
I think I figured this one out. However, if it's possible for users to encounter them, I suggest you consider making these diagnostics more informative. If they leave even a developer clueless, they can't be helping users much. -- Dave Abrahams Boost Consulting www.boost-consulting.com
Note that the documentation has an explanation of what this is under serialization/reference/special considerations/archive exceptions/unregistered_class I'm not sure what's missing from that explanation. Robert Ramey David Abrahams wrote:
David Abrahams <dave@boost-consulting.com> writes:
I really need the same kind of information about unregistered_class.
I think I figured this one out. However, if it's possible for users to encounter them, I suggest you consider making these diagnostics more informative. If they leave even a developer clueless, they can't be helping users much.
"Robert Ramey" <ramey@rrsd.com> writes:
Note that the documentation has an explanation of what this is under
serialization/reference/special considerations/archive exceptions/unregistered_class
I'm not sure what's missing from that explanation.
For users it's not bad, actually. For me, I wanted a programmer-level explanation, e.g. what function should have been called (or object constructed), and with what arguments? -- Dave Abrahams Boost Consulting www.boost-consulting.com
"Robert Ramey" <ramey@rrsd.com> writes:
Just tell me what you want me to do.
might as well just check 'em in to the trunk
Done. A few notes: * Borland 5.64 release build is crashing inside the test library. See my other posting today about that. Seems to be a codegen bug. * Metrowerks: I added an additional macro that lets MWerks users use BOOST_CLASS_EXPORT. // Everyone writes this BOOST_CLASS_EXPORT(polymorphic_derived1) // People targeting MWerks can do this BOOST_SERIALIZATION_MWERKS_BASE_AND_DERIVED(polymorphic_base, polymorphic_derived1) This makes the tests pass on cw-8.3 and cw-9.4. The regression table for Boost-1.33.1 shows the test_exported_xxxx tests passing on cw-8.1, but I was unable to get the CVS HEAD to pass cw-8.3 without the use of BOOST_SERIALIZATION_MWERKS_BASE_AND_DERIVED, so I figured it wasn't my responsibility to make my update work with that compiler. * It's unclear to me that BOOST_USED has any effect at all on whether the tests pass. I suggest defining it to be empty for a while and seeing what happens to the regression table. * I didn't run all the tests in every configuration; that's simply impractical. Robert, you need to make the library's testing more selective; it's occupying vastly more testing resources than most other libraries, and way more than required to effectively find problems. It shouldn't be necessary to test the cross-product of all tests with all archives and all build configurations. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes: * I didn't run all the tests in every configuration; that's simply impractical. Robert, you need to make the library's testing more selective; it's occupying vastly more testing resources than most other libraries, and way more than required to effectively find problems. It shouldn't be necessary to test the cross-product of all tests with all archives and all build configurations.
Actually, I'm thinking we should drop testing of serialization entirely. It just takes too long to do every day. Its useless to me since I only check in some change maybe once / month or so. And I do that only after running the complete test suite in both debug and release modes. I doubt that anyone else finds it useful. That is, if an error is detected because some interface breaking change occurs in another library, the other libraries author doesn't find out about it until I tell him. And I only check the results when I make some change. So it can't be much help to anyone else either. It is useful when someone adds a new compiler or something like that. But It would be just as good for the party interested in the compiler to run test complete serialization tests and forward to me the results. I don't need to install python or anything else. I just run bjam -test from a shell script and generate the results with an enhanced version of compiler_status (in the vault). In any case, this is also an infrequent occurence. This release cycle we've got a new borland compiler and a new STLPort platform. As for not running the cross product of the various combinations - the problem is that failures now occur in just a couple of select situations that can't be predicted in advance. BCB failed on text archives only in some cases. STLPort 5.0 failed when using mapped collections. etc. Paring down the test suite would likely have missed these failures. The selected tests would all pass - thereby giving the false impression that the library was bug free. Oh there's the issue of release mode. A number of compilers fail to instantiate code in release mode under some circumstances. Our current testing doesn't address that in any systematic way. I'm not even sure that the table shows whether the tests are run in release mode or debug mode. Then there is the issue of stability of the Boost test library. Its a fine job with lots of features. But when we started testing in January - I got a whole raft of failures because some compilers couldn't handle the test code. So I really need a simpler "plain vanilla - idiot proof - never fails" system. I've started making this change but its taking me some time to make the transition. So, I think your on the right track here - just take it to its logical conclusion. Robert Ramey
"Robert Ramey" <ramey@rrsd.com> writes:
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes: * I didn't run all the tests in every configuration; that's simply impractical. Robert, you need to make the library's testing more selective; it's occupying vastly more testing resources than most other libraries, and way more than required to effectively find problems. It shouldn't be necessary to test the cross-product of all tests with all archives and all build configurations.
Actually, I'm thinking we should drop testing of serialization entirely.
Wow, I don't know what to say. Is there _any_ standard Boost practice that should apply to this library? Or is it two libraries? <snip>
I'm not even sure that the table shows whether the tests are run in release mode or debug mode.
The answer is only a couple of clicks away in your web browser. <snip>
So, I think your on the right track here - just take it to its logical conclusion.
Sorry, what would that be? -- Dave Abrahams Boost Consulting www.boost-consulting.com
"Robert Ramey" <ramey@rrsd.com> writes:
As for not running the cross product of the various combinations - the problem is that failures now occur in just a couple of select situations that can't be predicted in advance. BCB failed on text archives only in some cases. STLPort 5.0 failed when using mapped collections. etc. Paring down the test suite would likely have missed these failures. The selected tests would all pass - thereby giving the false impression that the library was bug free.
Oh, there's a lot you can do to reduce it without losing any interesting data. For example, there is no reason to believe you're going to uncover serialization library bugs by testing against both static and dynamic runtimes, or especially by testing both singlethreaded and multithreaded builds, since you're not doing any threading in your tests. If you discover a test failure that appears only in one of these cases it is undoubtedly due to a compiler or standard library bug, not anything having to do with your code. Also, you could cut the testing time by 2/3 instantly if you'd just test all the archive types in a single executable, rather than building a separate executable for each one. And that would cover the case where multiple archives are in use, which I don't think you're covering today. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes:
As for not running the cross product of the various combinations - the problem is that failures now occur in just a couple of select situations that can't be predicted in advance. BCB failed on text archives only in some cases. STLPort 5.0 failed when using mapped collections. etc. Paring down the test suite would likely have missed these failures. The selected tests would all pass - thereby giving the false impression that the library was bug free.
Oh, there's a lot you can do to reduce it without losing any interesting data. For example, there is no reason to believe you're going to uncover serialization library bugs by testing against both static and dynamic runtimes,
I am testing for both static and dynamic runtimes.When creating the DLL versions and also auto-linking it was different. It is A LOT more than just recompiling. Now that things are working this is not necessary for debugging the serialization library itself.
or especially by testing both singlethreaded and multithreaded builds, since you're not doing any threading in your tests.
I don't believe I'm doing this. Currently, tests are built with the default threading model. That is, the Jamfile doesn't specify anything in this regard.
If you discover a test failure that appears only in one of these cases it is undoubtedly due to a compiler or standard library bug, not anything having to do with your code.
That was my original point. If I'm not making any changes, which I haven't been, almost no failures have anything to do with the serialization code.
Also, you could cut the testing time by 2/3 instantly if you'd just test all the archive types in a single executable, rather than building a separate executable for each one.
Hmmm - that was what I did originally but it was objected to in the first review. Also, more compilers failed with due to resource exhaustion. The current system has the benefit of permitting any user to test his own archive with all the tests. The documentation encourages users to do this with their own archives and I know they are doing it. I know this because when ever a user has a problem with his archive - I just tell him to run the whole test suite on it. The current system also lets me run just one archive by adding a command line argument - extremely useful. I actually didn't have any idea how compilation and execution time varies by making the separation. I had just presumed that compilation was more or less proportional to the number of archives - given that a lot of time is spent on serialization code instantiation which would be proportional to the number of archives. Other code is already compiled in the library. Truth is since I run while I'm not watching I don't know how the time would vary. How do you know this?
And that would cover the case where multiple archives are in use, which I don't think you're covering today.
see test_mult_archive_types In anycase, I have no problem with diminishing the the tests by just commenting out any combination of tests. Also the dynamic library versions can easily be suppressed by tweaking the Jamfile. As I said before, given that I'm making very few changes and I test those here, the boost test results are not used by me - or anyone else as far as I know. Robert Ramey
"Robert Ramey" <ramey@rrsd.com> writes:
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes:
As for not running the cross product of the various combinations - the problem is that failures now occur in just a couple of select situations that can't be predicted in advance. BCB failed on text archives only in some cases. STLPort 5.0 failed when using mapped collections. etc. Paring down the test suite would likely have missed these failures. The selected tests would all pass - thereby giving the false impression that the library was bug free.
Oh, there's a lot you can do to reduce it without losing any interesting data. For example, there is no reason to believe you're going to uncover serialization library bugs by testing against both static and dynamic runtimes,
I am testing for both static and dynamic runtimes.
I think you meant "not testing," which I see is true. You're testing against static and dynamic versions of your library.
When creating the DLL versions and also auto-linking it was different.
What was different?
It is A LOT more than just recompiling.
What is?
Now that things are working this is not necessary for debugging the serialization library itself.
or especially by testing both singlethreaded and multithreaded builds, since you're not doing any threading in your tests.
I don't believe I'm doing this. Currently, tests are built with the default threading model. That is, the Jamfile doesn't specify anything in this regard.
OK, maybe in my shock I overestimated. Sorry.
If you discover a test failure that appears only in one of these cases it is undoubtedly due to a compiler or standard library bug, not anything having to do with your code.
That was my original point. If I'm not making any changes, which I haven't been, almost no failures have anything to do with the serialization code.
But we want to find out when changes to other Boost libraries break it. And we want to find out when other people break it.
Also, you could cut the testing time by 2/3 instantly if you'd just test all the archive types in a single executable, rather than building a separate executable for each one.
Hmmm - that was what I did originally but it was objected to in the first review.
On what grounds?
Also, more compilers failed with due to resource exhaustion.
On some weaker compilers you might have to break a few tests into multiple translation units.
The current system has the benefit of permitting any user to test his own archive with all the tests. The documentation encourages users to do this with their own archives and I know they are doing it. I know this because when ever a user has a problem with his archive - I just tell him to run the whole test suite on it. The current system also lets me run just one archive by adding a command line argument - extremely useful.
I noticed. It's not only useful, but necessary, to cut down on the number of targets the Jamfile generates. All of that should be possible even with the tests built into a single executable.
I actually didn't have any idea how compilation and execution time varies by making the separation. I had just presumed that compilation was more or less proportional to the number of archives - given that a lot of time is spent on serialization code instantiation which would be proportional to the number of archives. Other code is already compiled in the library. Truth is since I run while I'm not watching I don't know how the time would vary. How do you know this?
I don't know for sure, but it's easy to see that every time you start a new compilation command there is overhead that can be avoided. Every time you include the same file in a new TU there's overhead that can be avoided. One eventually develops a pretty good instinct for these things after thinking about them a while.
And that would cover the case where multiple archives are in use, which I don't think you're covering today.
see test_mult_archive_types
In anycase, I have no problem with diminishing the the tests by just commenting out any combination of tests. Also the dynamic library versions can easily be suppressed by tweaking the Jamfile. As I said before, given that I'm making very few changes and I test those here, the boost test results are not used by me - or anyone else as far as I know.
The importance, to Boost as a whole, of having continuous testing of all libraries should not be underestimated. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes:
That was my original point. If I'm not making any changes, which I haven't been, almost no failures have anything to do with the serialization code.
But we want to find out when changes to other Boost libraries break it. And we want to find out when other people break it.
Which was my other point. Using one library(1) as a test on another library(2) isn't an efficient usage of resources. If this is deemed necessary its really an indication that the tests for library (2) have insufficient coverage and this should be fixed. Of course, I see the whole thing being tested before "release" but this would be an special occasion.
Also, you could cut the testing time by 2/3 instantly if you'd just test all the archive types in a single executable, rather than building a separate executable for each one.
Hmmm - that was what I did originally but it was objected to in the first review.
On what grounds?
If I remember correctly. that the tests where very large and complex, and not sufficiently focused.
Also, more compilers failed with due to resource exhaustion.
On some weaker compilers you might have to break a few tests into multiple translation units.
ahh - more work
The current system also lets me run just one archive by adding a command line argument - extremely useful.
I noticed. It's not only useful, but necessary, to cut down on the number of targets the Jamfile generates.
All of that should be possible even with the tests built into a single executable.
at the cost of making the test program more complex. And to make tests run faster in order to generate results that aren't very useful to me anyhow.
The importance, to Boost as a whole, of having continuous testing of all libraries should not be underestimated.
or overestimated. I'm not really objecting to testing - heck my main sin is creating "too many" tests and testing every combination which could possibly fail. (An XP maxim). In my view - other libraries don't have enough tests. Actually, everytime a bug slips through it should occasion adding a new test to the suite. But, constant testing using one library to test other libraries is a poor substitute for not adding more tests in the library that actually has the bug. Another issue is that we're not really testing our libraries - we're testing other compilers and standard libraries with our own code so we can add workarounds, jamfile tweaks, etc. I understand that this has worked well in the past but it isn't scaling well. I'm going to guess that as the "release" system evolves along lines being discussed, its going to percipitate a parallel evolution in the test system as well - we'll see. Robert Ramey
participants (3)
-
David Abrahams -
Rene Rivera -
Robert Ramey