- Boost - lists.boost.org

[endian] Review
by Stewart, Robert 14 Sep '11

14 Sep '11

Herewith is my review of Beman's Boost.Endian library. ____________________ ____________________ What is your evaluation of the design? _____ The two-argument overloads of the [un]conditional swapping functions seem awkward at first blush, but are really the best interface for those operations. Changing to value returning functions opens the door to mistakes like the following: uint32_t const in(123); int32_t const out(reorder(in)); There might be a warning, but many programmers routinely ignore such. As is, the compiler will complain about the conditional functions: uint32_t const in(123); int32_t out; native_to_big(in, out); // Error: ambiguous The unconditional functions do not provide that benefit because they aren't function templates: uint32_t const in(123); int32_t out; reorder(in, out); _____ An interesting variation that I've used in the past is a set of cast-style function templates that permit byte swapping one type and returning the result as a different type, after verifying that the resulting value is in range of the target type. Thus, uint32_t const in(std::numeric_limits<uint32_t>::max()); int32_t const out(to_host<int32_t>(in)); // throws TMP allows streamlining the range checks done, of course. _____ Another variation I've used is a checked, two-argument function template with each potentially having a different type: template <class T, class U> void to_host(T & _result, U _value); (I always prefer output parameters to be first because their position is consistent and it leaves open the possibility of defaulted parameters in the general case. I'd prefer that in Boost.Endian, but not everyone likes that convention.) There are two possibilities for dealing with the case when T and U are different. One is that they be required to have the same, supported size and that they are both signed or both unsigned. The other is to do a runtime check that the swapped value is within the range of the target type. _____ I like that the aligned integer types have the longer names because they incur padding and are non-portable. _____ The overhead implied by the comments in the "Comment on naming" section of the Endian Integer Types page is an excellent reason for splitting the endian class template into endian_buffer and endian_integer as previously suggested. For C++11, the latter can derive from the former. For C++03, if POD-ness is important, you'll need to duplicate the former's code in the latter, but it's worthwhile. ____________________ ____________________ What is your evaluation of the implementation? There's already been a good deal of discussion on efficiency and reuse during the review, so I won't rehash it. The 64b reorder(), for example, looks like a lot of code for an inline function. I haven't compiled it to inspect the resulting assembly to see how much or little results. My concern is for the possibility of code bloat arising from the desire to keep this a header-only library. If those were function templates, then the compiler can choose whether to inline or not, despite the definition being in a header. The implementation of class endian is uncommented, yet non-trivial. A future maintainer, even Beman, will benefit from some helpful information inserted now. The implementation of class endian would be simplified, it appears to me, by passing the endianness along to a detail::store() function template and letting it select the big/little endian store logic. ____________________ ____________________ What is your evaluation of the documentation? It's incomplete. There are no examples of the conversion functions. There is no tutorial. There is no guidance on selecting between the alternatives. There is no comparison with traditional htnol()-style code. __________ Endian Home _____ Introduction to endianness s/modern Apple, Linux, or Windows computer/computer/ (the OS doesn't matter) s|a[sic] Oracle/Sun Solaris computer| (the OS doesn't matter) s/CPU's/CPUs/ (plural, not possessive) _____ Introduction to the Boost.Endian library The first and last sentences should be merged: "Boost.Endian is a header only library providing facilities for managing integer endianness." Endian conversions for native integers: The last sentence makes both absolute and relative statements. The relative statements are, presumably, relative to endian integer types, but that's not made clear. I suggest rewording as, "This approach is simple and efficient, but when compared to endian integer types, is less flexible regarding size and alignment, can be harder to manage, is more error prone because of the need to apply endian conversions in unrelated logic paths, all of which can lead to logic errors that are difficult to debug." Endian integer types: The second sentence has a similar problem, and the last sentence could be combined to correspond with the list of the previous paragraph. Try this variation: "This approach is simple and, while it can be less efficient than endian conversions, it avoids strict alignment requirements allowing opportunities to pack data more tightly." I'd follow with this variation of the third sentence: "Furthermore, types of length from 1 to 8 bytes are supported, rather than just the usual 1, 2, 4, and 8 byte lengths." __________ Conversion Functions This page is odd. The synopsis does not include links to the description of the functions. When I first saw the page, I thought there simply was no documentation for using the various functions. That documentation does appear in the latter part of the page, but it isn't obvious. There should be information explaining why conditional and unconditional functions are useful and each should refer to its counterpart. That is, reorder(uint16_t &) should note that if the intent is not to reorder the bytes regardless of the platform, that bit_to_native(uint16_t &), for example, is appropriate. The unconditional nature or reorder() escaped me until I realized that these functions always swap bytes, unlike the htonl() family, which I'd never before encountered as necessary. Thus, use cases for unconditional byte swapping are warranted. __________ Endian Integer Types The Conversion Functions page links to the description of big and little endian on the Endian Home page, but the Integer Types page does not. The example shown on the Integer Types page should be recreated using the Conversion Functions to show the difference in usage. That example should use sizeof(h) rather than sizeof(header) since that is a safer practice. _____ Limitations This section, while important, should not appear so soon on the page. s/compilers do layout memory/compilers lay out memory/ s/C++0x/C++11/ s/it will be possible to specify the default constructor as trivial/the default constructor is marked as trivial/ s/will no longer disqualify/do not disqualify/ s/will no longer be relying/does not rely/ _____ Feature set s/Big endian | little endian | native endian byte ordering./Big, little, and native endian byte ordering/ s/Signed | unsigned/Signed and unsigned/ s/Unaligned | aligned/Data may be aligned or unaligned/ s/1-8 byte (unaligned) | 2, 4, 8 byte (aligned)/2, 4, and 8 byte aligned types and 1-8 byte unaligned types/ _____ Typedefs The column order should correspond with the order of information in the typedef names: alignment, sign, endianness, size. The discussion of the difference between aligned and unaligned types should not be "buried" in the Typedefs section. _____ Comment on naming s/these type grows/these types grows/ Why does "the realization creep in" that the endian integer types are lousy arithmetic integers? Only profiling would reveal that. Consequently, you should call that out, perhaps in a "Design Details" section along with the aligned/unaligned discussion, thereby explicitly stating rather than leaving to each library user the discovery of the overhead of these types. _____ Synopsis The link from "cover_operators" to a header is surprising. _____ Members The description of the default constructor fails to indicate whether the value is left uninitialized, which I presume to be the case. The description of the conversion operator mixes T and value_type. While those two are the same, one has to examine the class definition to see that. Either s/T/value_type/ or s/value_type/T/. _____ FAQ s/and both speed and/plus both speed and/ s/disk records/records/ (applies to networking, too) s/Yes, for sure, compared/There is overhead compared/ s/are usually be faster/are usually faster/ s/types POD's?/types POD?/ s/endian integer types not being POD's/of endian types not being POD/ s/data files are portable/data files that are portable/ The "These types are really just byte-holders" entry should not be a FAQ. That information, along with a discussion of the overhead of the simple syntax, should be part of the "Design Details" section suggested above. _____ Design considerations for Boost.Endian integers Only "Design" is underlined in the section heading. s/memcpyable/memcpy-able/ _____ Experience s/used very successful/used very successfully/ _____ Compilation s/This is ensures that , and/This ensures that class endian is POD, even in C++03, and/ s/POD's/POD/ ____________________ ____________________ What is your evaluation of the potential usefulness of the library? Highly useful, but could be more so. ____________________ ____________________ Did you try to use the library? No. Sorry. ____________________ ____________________ How much effort did you put into your evaluation? I spent about an hour reading the documentation plus a lot of time on the mailing list participating in endian discussions. ____________________ ____________________ Are you knowledgeable about the problem domain? Yes. I've used the old standby functions, htonl(), ntohl(), and friends over many years. I've created template functions to automatically select the right function based upon the size of the type being swapped, etc. ____________________ ____________________ Do you think the library should be accepted as a Boost library? There are issues that keep me from saying yes at this time. There are too many suggested variations and ideas under consideration to accept the library in its present state. However, a mini-review should be sufficient to evaluate the final form, once Beman determines a course of action, and determine whether to accept it or not. _____ Rob Stewart robert.stewart(a)sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com ________________________________ IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.

2 1

[proto] Looong compile times and other issues
by John Maddock 14 Sep '11

14 Sep '11

So.... I thought I had all the expression template stuff pretty much done, and then I tried some real world use cases (compiling all the students_t distribution functions with my extended-precision FP type) and everything fell apart: * VC10 wouldn't compile the code at all - more or less runs the system out of swap space (takes about 10 minutes or more!), then exits with an internal compiler error (if I break the code down into it's parts and instantiate each part separately it does compile though - that's not a solution though!). * GCC-4.4.x fails to compile the code due to clashes between boost::math::complement (a function) and boost::proto::complement (a class). I suspect this is an old gcc bug (finding structures via ADL) - I guess the solution is to not derive my number type from a proto-type so ADL can't find proto:: classes? Or will I hit this from some other unforeseen lookup? * GCC-4.5.0 Fails with an internal compiler error :-( * GCC-4.6.0 Builds the code OK, but takes a long time - though possibly just barely acceptable. I suspect this is a "triple template" problem: * proto is complex template library. * my number class is a fairly large/complex template in it's own right. * the code above gets instantiated from deep within Boost.Math's internals. Other than using proto::switch in the grammar (which I'm doing already), is there anything I can do to reduce the template load on the proto side of things? Otherwise since I'm only using a tiny fraction of proto's capabilities, the only other option I can see is to rip it out and replace with a mini-proto designed to minimalize template instantiations within this particular use case. Thanks in advance, John.

7 26

[C++11 Feature List]
by Sid Sacek 13 Sep '11

13 Sep '11

Does boost have a spec (document) where it mentions which new language features in C++11 are permitted to be used in new libraries? -Sid Sacek

2 2

[Endian] Performance
by Phil Endecott 13 Sep '11

13 Sep '11

Dear All, I've just done some quick benchmarks of Beman's proposed byte-swapping code using the following test program: #include <cstdint> #include <iostream> #include <arpa/inet.h> // for htonl() using std::uint32_t; static inline uint32_t byteswap(uint32_t src) { #if defined(USE_GCC_BUILTIN) return __builtin_bswap32(src); #elif defined(USE_HTONL) return htonl(src); #elif defined(USE_REV_INSTR) uint32_t target; __asm__ ( "rev %0, %1\n" : "=r" (target) : "r" (src) ); return target; #elif defined(USE_BEMAN) const char* s (reinterpret_cast<const char*>(&src)); uint32_t target; char * t (reinterpret_cast<char*>(&target) + sizeof(target) - 1); *t = *s; *--t = *++s; *--t = *++s; *--t = *++s; return target; #else #error "Define USE_*" #endif } int main() { uint32_t s = 0; for (uint32_t i = 0; i < 1000000000; ++i) { s += byteswap(i); } std::cout << s << "\n"; // Expect 506498560 } I've tested this on two systems: (A): Marvell ARMv5TE ("Feroceon") @ 1.2 GHz, g++ 4.4 (B): Freescale ARMv7 (i.MX53, Coretex A8) @ 1.0 GHz, g++ 4.6.1 Compiled with: --std=c++0x -O4 Execution times in seconds are: A B USE_GCC_BUILTIN 29.4 3.0 USE_HTONL 12.6 3.0 USE_REV_INSTR n/a 3.0 USE_BEMAN 17.6 8.0 ARM architecture version 6 (IIRC) introduced a "rev" byte-swapping instruction. On system B, which supports this, it is used by the gcc builtin and glibc's htonl() giving better performance than Beman's code. On system A which does not support this instruction, the gcc builtin makes a library call; I've not looked at what htonl() does. What do people see on other platforms? How important do we consider performance to be for this library? Regards, Phil.

11 37

[stopwatches] About reducing the scope of the library
by Vicente J. Botet Escriba 13 Sep '11

13 Sep '11

Hi, I'm not completly satisfied with the reporting interface/design I did for Boost.Stopwatches. I need to think more about the design so it can provide Locale facilities to the user. Do you think that it is worth proposing an addition to Boost.Chrono that provides only stopwatches (without reporting)? Best, Vicente

4 9

Re: [boost] [Submission] RawMemory (24-bit integers and runtime-dispatch)
by Adder 13 Sep '11

13 Sep '11

Hi there ! And thank you for your time and for your questions. Before I answer the first question regarding 24-bit integers, etc., please allow me to kindly ask you: Where are you going to store the 24-bit integer that you read from "raw memory" (= untyped memory = array of bytes) ? Or where is the 24-bit integer that you plan to write to "raw memory" stored ? (In what type of variable ?) Regarding the second question, regarding run-time dispatching of operations (as opposed to compile-time dispatching), I will start with a couple of examples that supports your question even more: It is not just the endian-ness that might not be known at compile-time. The alignment of data might also not be known before explicitly checking it at run-time. Let me quote from the ugly documentation that I have written (http://adder.iworks.ro/Boost/RawMemory/#UsersManual) "When we are dealing with an array of values (i.e. contiguous values) and performance is critical, we might want to check whether the data is aligned and specifically demand unchecked_memory_xfer if it is." Ultimately, the number of bytes that are used to represent the integer may be variable. Such is the case with the "mapping pair offsets" used by NTFS to locate the contiguous runs of clusters that make up the (non-MFT resident portion of the) file. In order to save precious MFT (Master File Table) entry space, signed offsets are stored in variable-sized (1 to 8-byte long) integers. My answer is probably obvious by now. I am thinking about letting the programmer use the ultimate weapon she has in her toolchest: The mighty "if" instruction. For the specific example you kindly gave, I am thinking that the following syntax might be useful for reading UCS-2 characters from a file saved with Notepad (in "Unicode" or "Unicode Big Endian" format): const uint16_t nByteOrderMask = raw_memory <uint16_t, little_endian>::peek (pFileBuffer); const bool bBigEndian = nByteOrderMask == 0xFEFF; ... // The loop: { const uint16_t cCurrent = bBigEndian ? raw_memory <uint16_t, big_endian>::peek (pCurrent) : raw_memory <uint16_t, little_endian>::peek (pCurrent); ... } I await your thoughts... Thank you again ! (-: -- Yours truly, Adder On 9/7/11, Joe Mucchiello <jmucchiello(a)yahoo.com> wrote: > For a library designed to deal with the weirdness of binary file formats, > I'm surprised it does not support 24-bit integers of arbitrary > alignment/sign. > >> uint32_t x = raw_memory <uint32_t, big_endian>::peek (&vbBuffer [5]); > > Do you also support a non-template "endianness" parameter? Sometimes you > don't know the endian-style of the file until you open it and read some > value out of it. > > Joe > > _______________________________________________ > Unsubscribe & other changes: > http://lists.boost.org/mailman/listinfo.cgi/boost >

2 5

[Endian] Review
by Tomas Puverle 12 Sep '11

12 Sep '11

Hi Beman, > - What is your evaluation of the design? The design of the integer types seems reasonable to me, even though I would prefer not to have the overloaded operators on them. I feel that having the operators has the potential to break the boundary between program input and the program's processing. What do you I mean by that? Typically, I try to structure my programs as follows: 1) Read input data 2) Verify input invariants, construct internal representation 3) Process internal representation 4) Output result The problem with having operators in the endian types is that they encourage the user to continue using them past the point 2). However, as I said, that's my personal preference and last year plenty of people wanted to see the operators included. Perhaps they could be a policy? endian<..., bool EnableOperators_ > ? As for the rest of the library, I have the following comments: - As many other people have mentioned, floating point support and user defined types are a must. - I am happy that the "conversion functions" have been added, Unfortunately, I find them lacking in several departments: - (Note: I am going to try very hard not to get drawn into another naming discussion. That's pretty much what ended my efforts last year - 50 people commenting, each proposing a different set of names :D ) - While the endian types support unaligned data, the endian functions don't appear to do so. I don't see how the endian types could be implemented using them. - No support for user defined types, arrays/ranges of user defined types etc. This is extremely useful. A frequent example I come across is the following (using our syntax): //load a file into memory //... swapInPlace<BigToNative>(data.begin(), data.end()); Depending on the platform, that last statement may get compiled out to a no-op. This is also the big reason why our version of "reorder()" is not unconditional but depends on the "swap type". - reorder(x) will work fine for integral types, however, has potential to cause issues for e.g. floating types. This has been discussed extensively during my proposal last year. Also, from my practical experience, having a floating point number as an input, I find the assembly typically includes (unncecessary) floating point load/store instructions. Something along the lines of e.g.: template<typename T_> T_ big_to_native(raw_memory<T_>) and template<typename T_> swapped_data<T_> native_to_big(T_) might be better and result in better performance. Of course, those two function signatures don't solve the "reorder" signature, but that could be changed correspondingly. - reorder() should use platform specific instructions (such as bswap) for doing endian swapping, otherwise falling back on one of the (many) variants which are currently being discussed. - I think the alternatives such as little_to_big() and big_to_little() are also needed. - I would like to see a few standard traits/typedefs so that I don't have to keep checking pre-processor macros, such as typedef boost::mpl::true_/false_ platform_is_big_endian; or something similar. - While I haven't needed this myself, last year someone mentioned the need for a "general reorder" functionality. Rather than simply dealing with little/big endianness, allow the user to specify ANY byte ordering for the specific data size. - I would like to see an endian_swap_iterator (and a raw_endian_iterator, which can iterate over unaligned data, too) > - What is your evaluation of the implementation? Other than the implementation of reorder() and its performance, which is already being discussed on the list, (and the inclusion of platform specific instructions, as per above) I would like to see endian<> to be implemented in terms of the "free" functions. It seems strange to have two implementations of the same task. > - What is your evaluation of the documentation? Fine. > - What is your evaluation of the potential usefulness of the > library? Very useful, long overdue in boost. > - Did you try to use the library? With what compiler? Did you > have any problems? Tried the integer types last year, no problems. Didn't try the "new" swapping functions. > - How much effort did you put into your evaluation? A glance? A > quick reading? In-depth study? I spent a fair bit of time looking through the docs, the code and reading the followup posts etc. > - Are you knowledgeable about the problem domain? Reasonably. > And finally, every review should answer this question: > > - Do you think the library should be accepted as a Boost library? I see the library as having two pieces, both useful to different people. I would say conditionally accept the endian<> class if floating point support is added. I would vote no for the inclusion of the free function interfaces in their current form. Thanks! Tom

6 14

[graph][iterators] csr graph and reverse iterator problem
by Takatoshi Kondo 11 Sep '11

11 Sep '11

Hi, All I'm using Boost.Graph and Boost.Iterators. When I use the compressed_sparse_row_graph with reverse_iterator, I ran into a problem. When I dereference the reverse_iterator of compressed_sparse_row_graph's iterator, invalid memory access occurs. On g++ version 4.6.0 with -Os option, The output is below. 0 1 1 0 0 1 134526756 3218710936 1 0 And I also checked where the invalid memory access occurred using valgrind. ==2341== Invalid read of size 4 ==2341== at 0x8048DE7: test_csr_graph() (rev_iter.cpp:45) ==2341== by 0x8048FB3: main (rev_iter.cpp:59) ==2341== Address 0xbe825f88 is just below the stack ptr. To suppress, use: --workaround-gcc296-bugs=yes ==2341== 1 ==2341== Invalid read of size 4 ==2341== at 0x8048E31: test_csr_graph() (rev_iter.cpp:46) ==2341== by 0x8048FB3: main (rev_iter.cpp:59) ==2341== Address 0xbe825f88 is just below the stack ptr. To suppress, use: --workaround-gcc296-bugs=yes ==2341== This result indicates the code below is suspicious. std::cout << *rib++ << std::endl; // NG Why? Line 45 std::cout << *rib++ << std::endl; // NG Why? Line 46 But... boost::tie(ib, ie) = boost::vertices(g); std::cout << *--ie << std::endl; // OK std::cout << *--ie << std::endl; // OK It is OK. Here is the source code that reproduces the problem. ------------ begin ------------- #include <boost/iterator/reverse_iterator.hpp> #include <boost/graph/adjacency_list.hpp> #include <boost/graph/compressed_sparse_row_graph.hpp> #include <iostream> #include <cassert> void test_graph() { typedef boost::adjacency_list<> graph_type; graph_type g(2); boost::graph_traits<graph_type>::vertex_iterator ib, ie; { boost::tie(ib, ie) = boost::vertices(g); std::cout << *ib++ << std::endl; // OK std::cout << *ib++ << std::endl; // OK assert(ib == ie); } { boost::tie(ib, ie) = boost::vertices(g); boost::reverse_iterator<boost::graph_traits<graph_type>::vertex_iterator> rib(ie); boost::reverse_iterator<boost::graph_traits<graph_type>::vertex_iterator> rie(ib); std::cout << *rib++ << std::endl; // OK std::cout << *rib++ << std::endl; // OK assert(rib == rie); } } void test_csr_graph() { typedef boost::compressed_sparse_row_graph<> graph_type; std::pair<int, int> e(0, 1); std::pair<int, int> es[] = { e }; graph_type g(boost::edges_are_unsorted, &es[0], &es[1], 2); boost::graph_traits<graph_type>::vertex_iterator ib, ie; { boost::tie(ib, ie) = boost::vertices(g); std::cout << *ib++ << std::endl; // OK std::cout << *ib++ << std::endl; // OK assert(ib == ie); } { boost::tie(ib, ie) = boost::vertices(g); boost::reverse_iterator<boost::graph_traits<graph_type>::vertex_iterator> rib(ie); boost::reverse_iterator<boost::graph_traits<graph_type>::vertex_iterator> rie(ib); std::cout << *rib++ << std::endl; // NG Why? Line 45 std::cout << *rib++ << std::endl; // NG Why? Line 46 assert(rib == rie); } { boost::tie(ib, ie) = boost::vertices(g); std::cout << *--ie << std::endl; // OK std::cout << *--ie << std::endl; // OK assert(ib == ie); } } int main() { test_graph(); test_csr_graph(); } ------------ end ------------- Why does the invalid memory access occur? Am I misusing the libraries? Thanks, Takatoshi

5 10

[type_erasure] ODR violation promotion
by Vicente J. Botet Escriba 10 Sep '11

10 Sep '11

Hi Steven, I'm interested in your TypeErasure library. Where can I find the on going version? The example for concept map seams not complete, as the specialization for less_than_comparable<std::type_info> is not used in any type erasure type. Could you complete it? Before the Boost.Conversion review someone signaled that the Boost.Conversion review was promoting ODR violations as two libraries could specialize a conversion for types S and T and make them incompatible for an end user as the ODR is violated. I think that your library suffer from the same issue, when two libraries need to specialize a concept provided by the library for a 3rd part concrete class using the proposed concept map mechanism. Do you agree? If not, how do the library avoids this kind of issue? Best, Vicente

3 10

[fusion] Algorithm/auxillary documentation
by Daniel James 10 Sep '11

10 Sep '11

Hi, The generated documentation for the algorithm/auxillary section hasn't been checked in. It should be at: http://svn.boost.org/svn/boost/trunk/libs/fusion/doc/html/fusion/algorithm/… http://svn.boost.org/svn/boost/trunk/libs/fusion/doc/html/fusion/algorithm/… Daniel

4 10