On Wed, Sep 7, 2011 at 1:03 PM, john filo <filo@arlut.utexas.edu> wrote:
- What is your evaluation of the design?
For the original scope (i.e. integers), very good. However, I'd like to see additional support for float, double, and user defined types added. Without it, this library isn't very useful to me personally since a large percentage of the values that I have to deal with are either float or double.
Several others have also expressed interest in FP, so I'll give it a try. Issues: * Because FP formats vary, just dealing with endianness doesn't ensure portability. * The endianness of FP and integer values differs on some platforms, so we will have to build up a config file with separate entries for each platform, and that will take time to mature. * Ditto FP sizes. * I'm only willing to provide conversion.hpp FP support. Providing types that mimic FP types is far beyond my knowledge of how to deal with floating point's notorious arithmetic issues.
---------------------------------------------------------------------- - What is your evaluation of the documentation?
Excellent.
Is a tutorial section going to be added? I'm not sure one is needed given the existing examples in the rest of the documentation, but the documentation's menu bar implies there will be one.
Yes, but it will be short and I won't have a chance to write it until after the formal review.
---------------------------------------------------------------------- - What is your evaluation of the implementation?
Very good.
At -O2, I was impressed with the performance relative to GCC's builtin byteswap routines. When I first saw the implementation, I honestly didn't think it would perform very well given its complexity compared to a naive implementation (e.g. the version used in the endian::reorder functions). I was pleasantly surprised. I guess that's a testament to the level of optimization compilers are capable of these days.
On an Intel Core2, the performance penalty of the uint32 endian<> template and the reorder() function relative to GCC-4.4 builtins (i.e. runtime divided by runtime of builtin) is as follows:
endian<> reorder() ------------------------ -O0 5.4 3.0 -O2 2.3 6.5
It seems like the reorder functions could be implemented in terms of the integer types for improved performance (and also maintainability). Something like:
template<typename T> inline void reorder(T& x) { x = *reinterpret_cast< endian<endianness::nonnative, T, sizeof(T)/8> *>(&x); }
template<typename T> inline void reorder(T source, T& target) { target = *reinterpret_cast< endian<endianness::nonnative, T, sizeof(T)/8> *>(&source); }
Obviously the above requires extending the endianess enum with a "nonnative" value.
Performance isn't that critical for me, but it does seem like providing platform specific implementations that take advantage of things like GCC's builtin byteswap functions should also be considered to get the performance up even further.
Finally, for my last performance related comment, I was surprised to see that the native types had basically exact same performance as the nonnative types. Couldn't the native types be specialized to effectively be a no-op?
There have been a lot of other suggestions about performance, and I'm actively working on performance issues right now. Stay tuned.
Would "byteswap" or maybe "swapbytes" be a better name than "reorder"?
I've tried a bunch of names, and am pretty happy with "reorder". But "swapbytes" would also be a decent name.
---------------------------------------------------------------------- - What is your evaluation of the potential usefulness of the library?
For those who deal with non-native endian data, this library is extremely useful. It automatically eliminates a whole class of common programming errors when dealing with such data.
---------------------------------------------------------------------- - Did you try to use the library? With what compiler?
Yes; gcc-4.4.3 on Linux
---------------------------------------------------------------------- - Are you knowledgeable about the problem domain?
Yes. I've used a similar home-grown library for the past 10 years, and have been looking forward this being added to Boost for a long time.
----------------------------------------------------------------------
- Do you think the library should be accepted as a Boost library?
Absolutely. I'd like to see support for float and double, but even without those additions, I still vote yes.
Thanks for the kind words, --Beman