[guid] - review results?
Hello Hartmut, Sorry about being pushy a bit, I'm just curious about the review results. I'd submit a review if I weren't moving from one country to another and defending my PhD during that week, sorry for that ;) -- Janek Kozicki |
Janek,
Sorry about being pushy a bit, I'm just curious about the review results.
I'd submit a review if I weren't moving from one country to another and defending my PhD during that week, sorry for that ;)
I was waiting for Andy to fix two of the major complaints raised during the review (incompatible license in parts of the code and the generator initialization seed issue), because these would have prevented the acceptance into Boost. As Andy told me, he is almost done with the fixes. As soon as these are finished I'll have a look at the code and will announce the review results. Regards Hartmut
Hartmut Kaiser skrev:
I was waiting for Andy to fix two of the major complaints raised during the review (incompatible license in parts of the code and the generator initialization seed issue), because these would have prevented the acceptance into Boost. As Andy told me, he is almost done with the fixes. As soon as these are finished I'll have a look at the code and will announce the review results.
What has these to do woth the review results? I mean, normally a library is accepted with a bunch of stuff that *must* be fixed before cvs upload. I simply don't understand how the current work can affect the review results? -Thorsten
Thorsten Ottosen wrote:
I was waiting for Andy to fix two of the major complaints raised during the review (incompatible license in parts of the code and the generator initialization seed issue), because these would have prevented the acceptance into Boost. As Andy told me, he is almost done with the fixes. As soon as these are finished I'll have a look at the code and will announce the review results.
What has these to do woth the review results? I mean, normally a library is accepted with a bunch of stuff that *must* be fixed before cvs upload.
I simply don't understand how the current work can affect the review results?
I understand your concerns and I was thinking a lot how to proceed. The complaints raised during the review have been very significant and would have prevented acceptance into Boost. OTOH, all of these have been discussed during the review exhaustively and AFAICS general agreement has been reached how to resolve the issues. Andy agreed on fixing the issues as discussed during the review in a short timeframe. My decision whether to reject or to tentatively accept the library depends on Andy's implementation. Since we need to have an additional mini review if I decide to tentatively accept the library anyway, I think it does no harm to wait for Andy before announcing the final decision. Regards Hartmut
Thorsten Ottosen wrote:
Hartmut Kaiser skrev:
I was waiting for Andy to fix two of the major complaints raised during the review (incompatible license in parts of the code and the generator initialization seed issue), because these would have prevented the acceptance into Boost. As Andy told me, he is almost done with the fixes. As soon as these are finished I'll have a look at the code and will announce the review results.
What has these to do woth the review results? I mean, normally a library is accepted with a bunch of stuff that *must* be fixed before cvs upload.
I think license issues are major obstacles and need to be resolved completely before any form of acceptance. IMHO, a library with license issues shouldn't even get accepted for review. Regards, m
"Hartmut Kaiser" <hartmut.kaiser@gmail.com> wrote in news:01fc01c7a168$9649acf0$0301a8c0@slartibartfast:
Janek,
Sorry about being pushy a bit, I'm just curious about the review results.
I'd submit a review if I weren't moving from one country to another and defending my PhD during that week, sorry for that ;)
I was waiting for Andy to fix two of the major complaints raised during the review (incompatible license in parts of the code and the generator initialization seed issue), because these would have prevented the acceptance into Boost. As Andy told me, he is almost done with the fixes. As soon as these are finished I'll have a look at the code and will announce the review results.
Regards Hartmut
I have uploaded a new version to the boost vault. uuid_v8.zip http://boost-consulting.com/vault/index.php? action=downloadfile&filename=uuid_v8.zip&directory=& The changes are: - no longer need Boost.Thread. All functions are thread safe - added create function that takes a random number generator - default random number generator is seeded better - removed static variables - implemented sha1 myself (no license problems) - removed byte_count() and output_bytes() - added size(), begin(), and end() - fixed constructor guid(ByteInputIterator, ByteInputIterator) - optimized operator>>() - optimized random number create function - renamed to uuid Andy.
I like the way you circumvent the license problem. Rewriting. Cool One question. One person reported that the uuid lib was quite slow in comparison to C# for instance. Could you resolve that, too? Thanks for your great work, Christian On 5/29/07, Andy <atompkins@fastmail.fm> wrote:
"Hartmut Kaiser" <hartmut.kaiser@gmail.com> wrote in news:01fc01c7a168$9649acf0$0301a8c0@slartibartfast:
Janek,
Sorry about being pushy a bit, I'm just curious about the review results.
I'd submit a review if I weren't moving from one country to another and defending my PhD during that week, sorry for that ;)
I was waiting for Andy to fix two of the major complaints raised during the review (incompatible license in parts of the code and the generator initialization seed issue), because these would have prevented the acceptance into Boost. As Andy told me, he is almost done with the fixes. As soon as these are finished I'll have a look at the code and will announce the review results.
Regards Hartmut
I have uploaded a new version to the boost vault. uuid_v8.zip http://boost-consulting.com/vault/index.php? action=downloadfile&filename=uuid_v8.zip&directory=&
The changes are: - no longer need Boost.Thread. All functions are thread safe - added create function that takes a random number generator - default random number generator is seeded better - removed static variables - implemented sha1 myself (no license problems) - removed byte_count() and output_bytes() - added size(), begin(), and end() - fixed constructor guid(ByteInputIterator, ByteInputIterator) - optimized operator>>() - optimized random number create function - renamed to uuid
Andy.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
"Christian Henning" <chhenning@gmail.com> wrote in news:949801310705291442q6067e19dsb0bd26722561b71f@mail.gmail.com:
I like the way you circumvent the license problem. Rewriting. Cool
Thanks :)
One question. One person reported that the uuid lib was quite slow in comparison to C# for instance. Could you resolve that, too?
I did optimize the random-based create function. It, of course, does depend on the random number generator used. The uuid::create(engine) (using mt19937) is now a little faster then .NET's. For 10,000,000 calls to uuid::create(engine), I get 3.937s, and for .NET's Guid::NewGuid(), I get 4.375s. The name-based uuid::create function has not been optimized. Here are the tests I used: #include "stdafx.h" #using <mscorlib.dll> using namespace System; int _tmain() { const int N = 10000000; DateTime t0 = DateTime::Now; for (int i = 0; i < N; i++) { Guid::NewGuid(); } TimeSpan ts = DateTime::Now - t0; Console::WriteLine(ts.ToString()); return 0; } and: #include <boost\progress.hpp> #include <boost\uuid.hpp> #include <iostream> #include <boost/random.hpp> int main(int argc, char* argv[]) { using namespace boost; const int N = 10000000; mt19937 engine; timer t1; for (int i=0; i<N; ++i) { uuid::create(engine); } std::cout << t1.elapsed() << '\n'; return 0; }
Thanks for your great work, Christian
< snip > Andy.
Sorry, this is a late review question. Has Microsoft GUID algorithm been published in any way? If it has, could we get an implementation of it? Typically when working on databases, one would not want to mix generated guids from different algorithms as that would compromise its uniqueness and thus increase collisions. So, if I was working on a Microsoft SQL Server, I would like to and may have to if an existing app with existing data, use the Microsoft GUID algorithm! -----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Andy Sent: Wednesday, May 30, 2007 10:01 AM To: boost@lists.boost.org Subject: Re: [boost] [guid] - review results? "Christian Henning" <chhenning@gmail.com> wrote in news:949801310705291442q6067e19dsb0bd26722561b71f@mail.gmail.com:
I like the way you circumvent the license problem. Rewriting. Cool
Thanks :)
One question. One person reported that the uuid lib was quite slow in comparison to C# for instance. Could you resolve that, too?
I did optimize the random-based create function. It, of course, does depend on the random number generator used. The uuid::create(engine) (using mt19937) is now a little faster then .NET's. For 10,000,000 calls to uuid::create(engine), I get 3.937s, and for .NET's Guid::NewGuid(), I get 4.375s. The name-based uuid::create function has not been optimized. Here are the tests I used: #include "stdafx.h" #using <mscorlib.dll> using namespace System; int _tmain() { const int N = 10000000; DateTime t0 = DateTime::Now; for (int i = 0; i < N; i++) { Guid::NewGuid(); } TimeSpan ts = DateTime::Now - t0; Console::WriteLine(ts.ToString()); return 0; } and: #include <boost\progress.hpp> #include <boost\uuid.hpp> #include <iostream> #include <boost/random.hpp> int main(int argc, char* argv[]) { using namespace boost; const int N = 10000000; mt19937 engine; timer t1; for (int i=0; i<N; ++i) { uuid::create(engine); } std::cout << t1.elapsed() << '\n'; return 0; }
Thanks for your great work, Christian
< snip > Andy. _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hello, some function definitions are missing the inline keyword (sha1.hpp and uuid.hpp). uuid::is_null() will no be renamed to uuid::is_nil() (ask in order to be conform to UUID-RFC)? Oliver
"Hartmut Kaiser" <hartmut.kaiser@gmail.com> wrote in news:01fc01c7a168$9649acf0$0301a8c0@slartibartfast:
Janek,
Sorry about being pushy a bit, I'm just curious about the review results.
I'd submit a review if I weren't moving from one country to another and defending my PhD during that week, sorry for that ;)
I was waiting for Andy to fix two of the major complaints raised during the review (incompatible license in parts of the code and the generator initialization seed issue), because these would have prevented the acceptance into Boost. As Andy told me, he is almost done with the fixes. As soon as these are finished I'll have a look at the code and will announce the review results.
Regards Hartmut
I have uploaded a new version to the boost vault. uuid_v8.zip http://boost-consulting.com/vault/index.php? action=downloadfile&filename=uuid_v8.zip&directory=&
The changes are: - no longer need Boost.Thread. All functions are thread safe - added create function that takes a random number generator - default random number generator is seeded better - removed static variables - implemented sha1 myself (no license problems) - removed byte_count() and output_bytes() - added size(), begin(), and end() - fixed constructor guid(ByteInputIterator, ByteInputIterator) - optimized operator>>() - optimized random number create function - renamed to uuid
Andy.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On 5/30/07, Oliver.Kowalke@qimonda.com <Oliver.Kowalke@qimonda.com> wrote:
Hello, some function definitions are missing the inline keyword (sha1.hpp and uuid.hpp). uuid::is_null() will no be renamed to uuid::is_nil() (ask in order to be conform to UUID-RFC)? Oliver
Personally, I'd stick to null over nil, as null is more the C++ standard (ie see the new 'null_ptr' that will probably be added to the next standard). Tony
"Gottlob Frege" <gottlobfrege@gmail.com> wrote in news:97ffb310705300624j611e0c54j56b256a127922268@mail.gmail.com:
On 5/30/07, Oliver.Kowalke@qimonda.com <Oliver.Kowalke@qimonda.com> wrote:
Hello, some function definitions are missing the inline keyword (sha1.hpp and uuid.hpp). uuid::is_null() will no be renamed to uuid::is_nil() (ask in order to be conform to UUID-RFC)? Oliver
Personally, I'd stick to null over nil, as null is more the C++ standard (ie see the new 'null_ptr' that will probably be added to the next standard).
I also perfer null, but I am happy to use what this list decides on. Maybe both?
Tony _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Andy.
Andy wrote:
uuid::is_null() will no be renamed to uuid::is_nil() (ask in order to be conform to UUID-RFC)?
I also perfer null, but I am happy to use what this list decides on. Maybe both?
I'd be fine with either name, but please don't provide both. I find it quite confusing if I have to choose how I want to call one and the same thing. Malte
Hi, <Oliver.Kowalke@qimonda.com> wrote in news:D0E7D90F1C8088459FBD399DE6D966FB56C566@drsse605.eu.infineon.com:
Hello, some function definitions are missing the inline keyword (sha1.hpp and uuid.hpp).
I will be happy to make these changes. Which functions?
uuid::is_null() will no be renamed to uuid::is_nil() (ask in order to be conform to UUID-RFC)?
I have not yet made all the changes mentioned during the review. I believe this was decided on.
Oliver
< snip > Andy.
Hello Andy,
some function definitions are missing the inline keyword (sha1.hpp and uuid.hpp).
I will be happy to make these changes. Which functions?
Each function definition in both files - you have separated the function declaration from definition and you put the definition also in the header. Without inlineing in the definition you get for instance "multiple definition of `boost::uuid::create()'" errors. uuid.hpp uuid uuid::create() : line 364 uuid uuid::create_random_based(Engine& engine) : line 374 std::basic_ostream<ch, char_traits>& operator<<(std::basic_ostream<ch, char_traits> &os, uuid const& u) : line 405 std::basic_istream<ch, char_traits>& operator>>(std::basic_istream<ch, char_traits> &is, uuid &u) : line 438 sha1.hpp unsigned int left_rotate(unsigned int x, size_t n) : line 26 void sha1::reset() : line 63 sha1::sha1() : line 58 void sha1::process_block() : line 100 void sha1::process_byte(unsigned char byte) : line 75 void sha1::process_block(void const* bytes_begin, void const* bytes_end) : line 85 void sha1::process_bytes(void const* buffer, std::size_t byte_count) : line 94 void sha1::get_digest(digest_type digest) : line 152 regards, Oliver
<Oliver.Kowalke@qimonda.com> wrote in news:D0E7D90F1C8088459FBD399DE6D966FB606B6E@drsse605.eu.infineon.com:
Hello Andy,
some function definitions are missing the inline keyword (sha1.hpp and uuid.hpp).
I will be happy to make these changes. Which functions?
Each function definition in both files - you have separated the function declaration from definition and you put the definition also in the header. Without inlineing in the definition you get for instance "multiple definition of `boost::uuid::create()'" errors.
uuid.hpp
uuid uuid::create() : line 364 uuid uuid::create_random_based(Engine& engine) : line 374 std::basic_ostream<ch, char_traits>& operator<<(std::basic_ostream<ch, char_traits> &os, uuid const& u) : line 405 std::basic_istream<ch, char_traits>& operator>>(std::basic_istream<ch, char_traits> &is, uuid &u) : line 438
sha1.hpp
unsigned int left_rotate(unsigned int x, size_t n) : line 26 void sha1::reset() : line 63 sha1::sha1() : line 58 void sha1::process_block() : line 100 void sha1::process_byte(unsigned char byte) : line 75 void sha1::process_block(void const* bytes_begin, void const* bytes_end) : line 85 void sha1::process_bytes(void const* buffer, std::size_t byte_count) : line 94 void sha1::get_digest(digest_type digest) : line 152
regards, Oliver _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Thanks! I added 'inline' to the non-template functions. I don't get the error for the templated functions. I can still add 'inline' to those if I should/need to. I am using visual studio .NET, maybe that makes a difference? I uploaded uuid_v9.zip to the boost vault. Will you give it a try for me? Thanks in advance, Andy.
Andy wrote:
I added 'inline' to the non-template functions. I don't get the error for the templated functions. I can still add 'inline' to those if I should/need to. I am using visual studio .NET, maybe that makes a difference?
I uploaded uuid_v9.zip to the boost vault. Will you give it a try for me?
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not. This instructs the compiler to treat this function to be visible inside the current translation unit only (even if the compiler decides not to inline the function), avoiding multiple symbol definitions if the header is included from more than one translation unit. Regards Hartmut
From: boost-bounces@lists.boost.org on behalf of Peter Dimov Sent: Thu 31/05/2007 18:04 To: boost@lists.boost.org Subject: Re: [boost] [guid] - review results? - inline missing Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true. ________________________________ <end quote> Why don't you think this is true? If you have the same function defined in two translation units (because it is dragged into both via #include), it's a violation of the One Definition Rule unless the definitions have inline. Apologies for the crap quoting - blame Exchange Webmail
On 5/31/07, Martin Bonner <Martin.Bonner@pi-shurlok.com> wrote:
From: boost-bounces@lists.boost.org on behalf of Peter Dimov Sent: Thu 31/05/2007 18:04 To: boost@lists.boost.org Subject: Re: [boost] [guid] - review results? - inline missing
Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true. ________________________________
<end quote> Why don't you think this is true?
If you have the same function defined in two translation units (because it is dragged into both via #include), it's a violation of the One Definition Rule unless the definitions have inline.
Or unless it's a template function, which is what I'm almost sure Peter was referring to. --Michael Fawcett
"Martin Bonner" <Martin.Bonner@pi-shurlok.com> wrote in news:5D8503D66B71984E8D7F6651BA74D0592E0071@yew.PS.LOCAL:
From: boost-bounces@lists.boost.org on behalf of Peter Dimov Sent: Thu 31/05/2007 18:04 To: boost@lists.boost.org Subject: Re: [boost] [guid] - review results? - inline missing
Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true. ________________________________
<end quote> Why don't you think this is true?
If you have the same function defined in two translation units (because it is dragged into both via #include), it's a violation of the One Definition Rule unless the definitions have inline.
< snip > I don't know what is correct by the standard, or what is correct to do to ensure that all comilers will be able to compile it. But, I included uuid.hpp (from uuid_v9.zip that I just uploaded to the vault) in two different complication units (and called some of the templated functions). The templated functions do not have the inline keyword and I don't get a complication error using visual studio .NET. Andy.
----Original Message---- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Andy Sent: 31 May 2007 18:43 To: boost@lists.boost.org Subject: Re: [boost] [guid] - review results? - inline missing
"Martin Bonner" <Martin.Bonner@pi-shurlok.com> wrote in news:5D8503D66B71984E8D7F6651BA74D0592E0071@yew.PS.LOCAL:
From: boost-bounces@lists.boost.org on behalf of Peter Dimov Sent: Thu 31/05/2007 18:04 To: boost@lists.boost.org Subject: Re: [boost] [guid] - review results? - inline missing
Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true. ________________________________
<end quote> Why don't you think this is true?
If you have the same function defined in two translation units (because it is dragged into both via #include), it's a violation of the One Definition Rule unless the definitions have inline.
< snip >
I don't know what is correct by the standard, or what is correct to do to ensure that all comilers will be able to compile it.
But, I included uuid.hpp (from uuid_v9.zip that I just uploaded to the vault) in two different complication units (and called some of the templated functions). The templated functions do not have the inline keyword and I don't get a complication error using visual studio .NET.
See my other post (executive summary, Peter is right and I was wrong), but not getting a compilaton error is not much evidence. The normal way that violations of the ODR manifest themselves is with a linker error, or unexpected behaviour at runtime because what the programmer thought was a single object has ended up in two places. -- Martin Bonner Project Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com
From: Martin Bonner
From: Peter Dimov Sent: Thu 31/05/2007 18:04
Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true.
Why don't you think this is true?
If you have the same function defined in two translation units (because it is dragged into both via #include), it's a violation of the One Definition Rule unless the definitions have inline.
Apologies for the crap quoting - blame Exchange Webmail
OK. I've fixed the crap quoting. Now it's time to apologize for the crap content. Specifically, section 3.2 [one definition rule] paragraph 5 says: "There can be more than one definition of a ... non-static function template ... in a program provided that each definition appears in a different translation unit, and provided the definitions satisfy the following requirements." (The requirements are met by a definition dragged into two translation units via #include.) In other words Peter is right; you don't need to declare a function template as "inline". If you do, the only effect may be to encourage the compiler to inline it. -- Martin Bonner Project Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com <mailto:martin.bonner@pi-shurlok.com> www.pi-shurlok.com <http://www.pi-shurlok.com/>
"Martin Bonner" <Martin.Bonner@pi-shurlok.com> wrote in news:5D8503D66B71984E8D7F6651BA74D059414E49@yew.PS.LOCAL:
From: Martin Bonner
From: Peter Dimov Sent: Thu 31/05/2007 18:04
Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true.
Why don't you think this is true?
If you have the same function defined in two translation units (because it is dragged into both via #include), it's a violation of the One Definition Rule unless the definitions have inline.
Apologies for the crap quoting - blame Exchange Webmail
OK. I've fixed the crap quoting. Now it's time to apologize for the crap content. Specifically, section 3.2 [one definition rule] paragraph 5 says:
"There can be more than one definition of a ... non-static function template ... in a program provided that each definition appears in a different translation unit, and provided the definitions satisfy the following requirements."
(The requirements are met by a definition dragged into two translation units via #include.)
In other words Peter is right; you don't need to declare a function template as "inline". If you do, the only effect may be to encourage the compiler to inline it.
Thanks. Andy.
Peter, Your answer isn't very helpful. Do you mind elaborating? Regards Hartmut
Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true. _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hartmut Kaiser wrote:
Peter,
Your answer isn't very helpful. Do you mind elaborating?
Regards Hartmut
Hartmut Kaiser wrote:
You need to add an explicit 'inline' specifier to any function defined out of class but in a header, regardless if it's a templatized function or not.
I don't think that this is true.
Templates do not need to be declared inline, unless one wants them inlined (this is compiler and option dependent; some compilers honor the lack of inline and refrain from inlining, others do not).
Hi,
I added 'inline' to the non-template functions. I don't get the error for the templated functions. I can still add 'inline' to those if I should/need to. I am using visual studio .NET, maybe that makes a difference?
I uploaded uuid_v9.zip to the boost vault. Will you give it a try for me?
The new version works but I get still this error: /opt/boost/include/boost-1_34/boost/uuid.hpp:210: instantiated from void boost::uuid::construct(const std::basic_string<_CharT, _Traits, _Alloc>&) [with ch = char, char_traits = std::char_traits<char>, alloc = std::allocator<char>] /opt/boost/include/boost-1_34/boost/uuid.hpp:77: instantiated from here /opt/boost/include/boost-1_34/boost/uuid.hpp:469: error: function find(char [16], char* const&, char&) not found Any ideas? Regards, Oliver
participants (13)
-
Andy -
Christian Henning -
Gottlob Frege -
Hartmut Kaiser -
Janek Kozicki -
Jarrad Waterloo -
Malte Clasen -
Martin Bonner -
Martin Wille -
Michael Fawcett -
Oliver.Kowalke＠qimonda.com -
Peter Dimov -
Thorsten Ottosen