Boost.Uuid and header-only support

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
49 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Boost.Uuid and header-only support

Boost - Dev mailing list
Hi folks,

I have an open defect related to a random seed generator that's currently
in boost::uuids::detail called seed_rng.  The class does a fair bit more
than it really needs to, and does not support Universal Windows Platform
(UWP) store targets.  It is otherwise quite similar to
boost::random::random_device however it is a header-only implementation (in
fact seed_rng has a comment about putting it into Boost.Random).  I was
planning on removing it and leaving randomness to Boost.Random because
that's where the code really belongs, however this would introduce a new
library dependency on Boost.Random for any code using
boost::uuids::random_generator.  So I'm curious on how important folks
believe it is to maintain the header-only status of Boost.Uuid.

Thanks,

Jim

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
James E. King, III wrote:

> Hi folks,
>
> I have an open defect related to a random seed generator that's currently
> in boost::uuids::detail called seed_rng.  The class does a fair bit more
> than it really needs to, and does not support Universal Windows Platform
> (UWP) store targets.  It is otherwise quite similar to
> boost::random::random_device however it is a header-only implementation
> (in fact seed_rng has a comment about putting it into Boost.Random).  I
> was planning on removing it and leaving randomness to Boost.Random because
> that's where the code really belongs, however this would introduce a new
> library dependency on Boost.Random for any code using
> boost::uuids::random_generator.  So I'm curious on how important folks
> believe it is to maintain the header-only status of Boost.Uuid.

You had plans of contributing a header-only random_device to Boost.Random, I
think?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Fri, Nov 3, 2017 at 7:58 PM, James E. King, III wrote:

> I have an open defect related to a random seed generator that's currently
> in boost::uuids::detail called seed_rng.  The class does a fair bit more
> than it really needs to, and does not support Universal Windows Platform
> (UWP) store targets.  It is otherwise quite similar to
> boost::random::random_device however it is a header-only implementation (in
> fact seed_rng has a comment about putting it into Boost.Random).  I was
> planning on removing it and leaving randomness to Boost.Random because
> that's where the code really belongs, however this would introduce a new
> library dependency on Boost.Random for any code using
> boost::uuids::random_generator.  So I'm curious on how important folks
> believe it is to maintain the header-only status of Boost.Uuid.

While I don't use Boost.Uuid, I do think users of Boost enjoy the
convenience of header-only libraries (and know of at some who are
restricted to - within their organization - only the header-only
libraries). Why not make Boost.Random header-only first, before taking
the dependency on it?

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 11/04/17 03:15, Peter Dimov via Boost wrote:

> James E. King, III wrote:
>> Hi folks,
>>
>> I have an open defect related to a random seed generator that's
>> currently in boost::uuids::detail called seed_rng.  The class does a
>> fair bit more than it really needs to, and does not support Universal
>> Windows Platform (UWP) store targets.  It is otherwise quite similar
>> to boost::random::random_device however it is a header-only
>> implementation (in fact seed_rng has a comment about putting it into
>> Boost.Random).  I was planning on removing it and leaving randomness
>> to Boost.Random because that's where the code really belongs, however
>> this would introduce a new library dependency on Boost.Random for any
>> code using boost::uuids::random_generator.  So I'm curious on how
>> important folks believe it is to maintain the header-only status of
>> Boost.Uuid.
>
> You had plans of contributing a header-only random_device to
> Boost.Random, I think?

The proposal to make Boost.Random header-only was rejected by Steven.

https://github.com/boostorg/random/pull/29

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 11/04/17 02:58, James E. King, III via Boost wrote:

> Hi folks,
>
> I have an open defect related to a random seed generator that's currently
> in boost::uuids::detail called seed_rng.  The class does a fair bit more
> than it really needs to, and does not support Universal Windows Platform
> (UWP) store targets.  It is otherwise quite similar to
> boost::random::random_device however it is a header-only implementation (in
> fact seed_rng has a comment about putting it into Boost.Random).  I was
> planning on removing it and leaving randomness to Boost.Random because
> that's where the code really belongs, however this would introduce a new
> library dependency on Boost.Random for any code using
> boost::uuids::random_generator.  So I'm curious on how important folks
> believe it is to maintain the header-only status of Boost.Uuid.

I've expressed my opinion to you privately and will reproduce it here
just for the record.

I think the header-only property of Boost.UUID is an important feature
that enables some users to use the library. Whether that is due to a
project/company policy, pure convenience or other reasons is irrelevant;
the important part is that some users simply won't use separately
compiled libraries. Transiting to non-header-only state puts those users
in an awkward position, having to either port away from Boost.UUID
(likely, to a home-grown solution) or to adjust their policies and build
environment. Neither of those are good and easy outcomes.

Depending on how exactly you want to implement this, adding linking
dependency on Boost.Random can break POSIX users as autolinking is not
available on POSIX. (I.e. POSIX can be saved if you keep the current
header-only code for POSIX and only limit your change to Windows.)
Although autolinking can somewhat hide the problem on Windows, I don't
consider it a solution because it won't help if the compiled libraries
are simply not built.

With the Boost.Random proposal rejected, I can see the following
possible solutions:

1. Add the bcrypt-based implementation to the existing ones in
seed_rng.hpp. I think, this is the simplest to implement and it will
keep the users happy. Personally, I would do this.

2. Add your header-only random_device implementation proposed for
Boost.Random as an implementation detail to Boost.UUID and use it
internally.

3. Use std::random on newer compilers on Windows. Arguably,
std::random_device is supported on UWP as well as other Windows flavors.
Keep the current code for all other platforms and older compilers on
Windows.

4. Use std::random universally on C++11 and later and Boost.Random on
C++03. This will break C++03 users but will maintain the header-only
property for C++11 and on.

Personally, I don't see the point in forcing the use of std::random or
Boost.Random. It looks nice from the maintainer's point of view (i.e.
separate concerns and let the random library deal with RNG), but given
that most of the code is already written in Boost.UUID and forcing
Boost.Random is actively harmful to Boost.UUID it is not worth it.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Andrey Semashev wrote:

> The proposal to make Boost.Random header-only was rejected by Steven.
>
> https://github.com/boostorg/random/pull/29

This sounds like essential context that might have been worth mentioning.
:-)

Steven is right that we're still dependent on Boost.System, this needs to be
solved somehow.

To get back to Uuid. The whole seed_rng file needs to be retired. It's only
an implementation detail of a primitive operation that returns N random
bytes. At the time seed_rng was written, there was no good way of obtaining
high quality randomness from the OS, but this is not the case today.

What the Uuid library needs, when generating a random UUID, is just an array
of random bytes; there's no need to create a random number generator, then
seed it. None at all. This is historical baggage.

And in my opinion, all the conceptification and templatization in James's PR
is completely unnecessary and just adds complexity. What we need is this:

namespace boost
{
    int get_random_bytes( void * buffer, size_t size ); // returns errno
}

This happens to match the Linux function with the same name but this is
coincidental.

The implementation of this function needs to

- read from /dev/urandom on POSIX
- use RtlGenRandom on Windows desktop
- use Bcrypt on Windows non-desktop

We have to decide where to put it though. Utility springs to mind as a first
candidate.

Switching topics back to Boost.Random, I consider the use of the token to
select a cryptographic provider a bad practice. Even the POSIX path should
ignore the token. get_random_bytes is the way to go. But that's a separate
story.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
On 11/04/17 18:40, Peter Dimov via Boost wrote:

> Andrey Semashev wrote:
>
>> The proposal to make Boost.Random header-only was rejected by Steven.
>>
>> https://github.com/boostorg/random/pull/29
>
> This sounds like essential context that might have been worth
> mentioning. :-)
>
> Steven is right that we're still dependent on Boost.System, this needs
> to be solved somehow.

I think, Steven was more opposed to the whole idea of header-only code,
regardless of implementation.

> To get back to Uuid. The whole seed_rng file needs to be retired. It's
> only an implementation detail of a primitive operation that returns N
> random bytes. At the time seed_rng was written, there was no good way of
> obtaining high quality randomness from the OS, but this is not the case
> today.
>
> What the Uuid library needs, when generating a random UUID, is just an
> array of random bytes; there's no need to create a random number
> generator, then seed it. None at all. This is historical baggage.

Well, not exactly. I agree that some code in seed_rng.hpp is
unneccessary and outdated. But certainly not all of it.

Boost.UUID needs a good PRNG, preferably a fast one. An RNG that blocks
if there's not enough entropy is not suitable. If OS provides such PRNG
then great, we can use it. I'm not sure every platform does, though. In
particular, I'm not sure Windows APIs guarantee the non-blocking behavior.

Another point to consider is whether we want to deplete system entropy
pool when generating lots of UUIDs. The primary property we need from
random numbers is uniqueness; we don't require cryprographical
randomness for every byte of an UUID or maybe even every generated UUID.

> And in my opinion, all the conceptification and templatization in
> James's PR is completely unnecessary and just adds complexity. What we
> need is this:
>
> namespace boost
> {
>     int get_random_bytes( void * buffer, size_t size ); // returns errno
> }
>
> This happens to match the Linux function with the same name but this is
> coincidental.
>
> The implementation of this function needs to
>
> - read from /dev/urandom on POSIX

There are also Linux-specific getrandom and BSD-specific getentropy.

> - use RtlGenRandom on Windows desktop

IIRC, it's only available since Vista or 7? I forget.

> - use Bcrypt on Windows non-desktop

The devil is in the details, as usually. The function is not the proper
interface for such a tool because, e.g. in case of /dev/urandom-based
implementation, it will have to open/close a file on every call. This is
an additional source of failure and an attack vector.

The standard interface for system RNG is std::random_device, which you
can create and use multiple times. Boost.UUID also rightfully implements
random_generator as an object to accommodate this usage pattern.

> Switching topics back to Boost.Random, I consider the use of the token
> to select a cryptographic provider a bad practice. Even the POSIX path
> should ignore the token. get_random_bytes is the way to go. But that's a
> separate story.

I'm not sure I understand what you mean here. I agree that APIs like
getrandom and getentropy are superior to reading from /dev/urandom, but
those are not available universally. I'm not sure how that relates to
Windows though.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
On 11/4/2017 12:57 PM, Andrey Semashev via Boost wrote:

> On 11/04/17 18:40, Peter Dimov via Boost wrote:
>> Andrey Semashev wrote:
>>
>>> The proposal to make Boost.Random header-only was rejected by Steven.
>>>
>>> https://github.com/boostorg/random/pull/29
>>
>> This sounds like essential context that might have been worth
>> mentioning. :-)
>>
>> Steven is right that we're still dependent on Boost.System, this needs
>> to be solved somehow.
>
> I think, Steven was more opposed to the whole idea of header-only code,
> regardless of implementation.

Steven can chime in if he wants but my interpretation was that Steven
was rightly concerned with the idea that making Boost random header-only
only works if the end-user is forced to use Boost system as header-only,
and we do not want to tell the end-user what he must do.

snipped...


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Sat, Nov 4, 2017 at 11:40 AM, Peter Dimov via Boost <
[hidden email]> wrote:

> Andrey Semashev wrote:
>
> The proposal to make Boost.Random header-only was rejected by Steven.
>>
>> https://github.com/boostorg/random/pull/29
>>
>
> This sounds like essential context that might have been worth mentioning.
> :-)
>
>
I specifically did not mention the PR resolution because my intention was
to find a path forward, not to bring attention to that decision.

Steven is right that we're still dependent on Boost.System, this needs to
> be solved somehow.
>
> To get back to Uuid. The whole seed_rng file needs to be retired. It's
> only an implementation detail of a primitive operation that returns N
> random bytes. At the time seed_rng was written, there was no good way of
> obtaining high quality randomness from the OS, but this is not the case
> today.
>

boost::uuids::detail::seed_rng has the following problems:

1. It duplicates the behavior of boost::random_device.
2. It is unnecessarily complex (does way more than boost::random_device).
3. It ignores errors, leading potentially to not properly seeding the RNG.
4. It has no support for Universal Windows Platform.


> What the Uuid library needs, when generating a random UUID, is just an
> array of random bytes; there's no need to create a random number generator,
> then seed it. None at all. This is historical baggage.
>
> And in my opinion, all the conceptification and templatization in James's
> PR is completely unnecessary and just adds complexity. What we need is this:

namespace boost

> {
>    int get_random_bytes( void * buffer, size_t size ); // returns errno
> }
>
> This happens to match the Linux function with the same name but this is
> coincidental.
>
> The implementation of this function needs to
>
> - read from /dev/urandom on POSIX
> - use RtlGenRandom on Windows desktop
> - use Bcrypt on Windows non-desktop
>
>
It would be safer to use Wincrypt on windows desktop for these reasons:

1. RtlGenRandom has no published interface and may disappear at any time.
2. Consumers who use their own Wincrypt provider for entropy would be
broken by such a change.


> We have to decide where to put it though. Utility springs to mind as a
> first candidate.
>

Something called "get_random_bytes" seems like it would belong in
Boost.Random...


> Switching topics back to Boost.Random, I consider the use of the token to
> select a cryptographic provider a bad practice. Even the POSIX path should
> ignore the token. get_random_bytes is the way to go. But that's a separate
> story.


The token allows the POSIX implementation to use a different entropy
provider device (other than /dev/urandom) or even a text file.
There may be reasons why folks use their own, for example in the government
sector, so any change in that area would be a breaking change.

Thanks,

- Jim

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
James E. King, III wrote:
> It would be safer to use Wincrypt on windows desktop for these reasons:
>
> 1. RtlGenRandom has no published interface...

https://msdn.microsoft.com/en-us/library/windows/desktop/aa387694(v=vs.85).aspx

> and may disappear at any time.

If it does, we'll deal with it.

> 2. Consumers who use their own Wincrypt provider for entropy would be
> broken by such a change.

First, there are no such consumers, and second, get_random_bytes has no way
to supply a "provider", by design.

> The token allows the POSIX implementation to use a different entropy
> provider device (other than /dev/urandom)...

I know. This is wrong and should never be used.

>  or even a text file.

random_device is not the correct or portable way to use a text file as a
source of random numbers.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Sat, Nov 4, 2017 at 1:16 PM, Edward Diener via Boost <
[hidden email]> wrote:

> On 11/4/2017 12:57 PM, Andrey Semashev via Boost wrote:
>
>> On 11/04/17 18:40, Peter Dimov via Boost wrote:
>>
>>> Andrey Semashev wrote:
>>>
>>> The proposal to make Boost.Random header-only was rejected by Steven.
>>>>
>>>> https://github.com/boostorg/random/pull/29
>>>>
>>>
>>> This sounds like essential context that might have been worth
>>> mentioning. :-)
>>>
>>> Steven is right that we're still dependent on Boost.System, this needs
>>> to be solved somehow.
>>>
>>
>> I think, Steven was more opposed to the whole idea of header-only code,
>> regardless of implementation.
>>
>
> Steven can chime in if he wants but my interpretation was that Steven was
> rightly concerned with the idea that making Boost random header-only only
> works if the end-user is forced to use Boost system as header-only, and we
> do not want to tell the end-user what he must do.
>

Making Boost.Random header-only could still be considered an improvement,
whether or not what it depends on is header-only.
Perhaps defining BOOST_ERROR_CODE_HEADER_ONLY would allow someone to use
both libraries in a header-only environment?  I'm not sure.

Given that random_device is the only implementation that uses system_error,
one could make the errors in random_device more abstract with its own
exception type, i.e. boost::random::entropy_error : public
std::runtime_error, and sever the link between the libraries, and then a
header-only implementation of random would be immediately useful...

- Jim

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
James E. King, III wrote:

> Making Boost.Random header-only could still be considered an improvement,
> whether or not what it depends on is header-only.

It might, but Uuid will still regress from header-only to requiring a
library, so this doesn't help.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
On Sat, Nov 4, 2017 at 1:53 PM, Peter Dimov via Boost <[hidden email]
> wrote:

> James E. King, III wrote:
>
> Making Boost.Random header-only could still be considered an improvement,
>> whether or not what it depends on is header-only.
>>
>
> It might, but Uuid will still regress from header-only to requiring a
> library, so this doesn't help.
>
>
I was wondering, does it even make sense to have the default RNG of
uuids::random_generator
set to a PseudoRandomNumberGenerator for boost::uuid? It is very expensive
to seed and then use
a mersenne twister when compared to just acquiring 16 bytes of entropy.

I'd like the default uuids::random_generator to use a random_device
implementation (I have
successfully moved the implementation I have into uuid detail locally),
however if I change
the default type in uuids::random_generator that would be a breaking change
for
anyone using the constructors that take a reference or a pointer... it
would be much more efficient
to just get 16 bytes of entropy instead.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
James E. King, III wrote:
> I was wondering, does it even make sense to have the default RNG of
> uuids::random_generator set to a PseudoRandomNumberGenerator for
> boost::uuid?

No, in my opinion it doesn't. basic_random_generator has to be retained for
compatibility, but random_generator should just obtain random bytes
directly. You're right that this is a breaking change though - a justified
one, in my opinion.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
On Sat, Nov 4, 2017 at 8:53 PM, Peter Dimov via Boost <[hidden email]
> wrote:

> James E. King, III wrote:
>
>> I was wondering, does it even make sense to have the default RNG of
>> uuids::random_generator set to a PseudoRandomNumberGenerator for
>> boost::uuid?
>>
>
> No, in my opinion it doesn't. basic_random_generator has to be retained
> for compatibility, but random_generator should just obtain random bytes
> directly. You're right that this is a breaking change though - a justified
> one, in my opinion.
>
>
Looks like November 1 was the deadline for making major changes for 1.66.0,
which I assume would include breaking changes...

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
James E. King, III wrote:

> On Sat, Nov 4, 2017 at 8:53 PM, Peter Dimov via Boost
> <[hidden email] wrote:
>
> > James E. King, III wrote:
> >
> >> I was wondering, does it even make sense to have the default RNG of
> >> uuids::random_generator set to a PseudoRandomNumberGenerator for
> >> boost::uuid?
> >>
> >
> > No, in my opinion it doesn't. basic_random_generator has to be retained
> > for compatibility, but random_generator should just obtain random bytes
> > directly. You're right that this is a breaking change though - a
> > justified one, in my opinion.
> >
>
> Looks like November 1 was the deadline for making major changes for
> 1.66.0, which I assume would include breaking changes...

That's true, in principle.

The intent behind the rule is however that this refers to changes that break
other Boost libraries, not client code outside of Boost. Client code
typically sees the breaking change after the release, or at best after the
beta, not before. So breaking client code does not in general impede the
release process, whereas breaking other Boost libraries (or otherwise
introducing regressions into the testing and release procedures) most
certainly can and does.

But a permission from a release manager cannot hurt if you decide to pursue
this.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
On Sat, Nov 4, 2017 at 9:37 PM, Peter Dimov via Boost <[hidden email]
> wrote:

> James E. King, III wrote:
>
>> On Sat, Nov 4, 2017 at 8:53 PM, Peter Dimov via Boost <
>> [hidden email] wrote:
>>
>> > James E. King, III wrote:
>> >
>> >> I was wondering, does it even make sense to have the default RNG of >>
>> uuids::random_generator set to a PseudoRandomNumberGenerator for >>
>> boost::uuid?
>> >>
>> >
>> > No, in my opinion it doesn't. basic_random_generator has to be retained
>> > for compatibility, but random_generator should just obtain random bytes >
>> directly. You're right that this is a breaking change though - a >
>> justified one, in my opinion.
>> >
>>
>> Looks like November 1 was the deadline for making major changes for
>> 1.66.0, which I assume would include breaking changes...
>>
>
> That's true, in principle.
>
> The intent behind the rule is however that this refers to changes that
> break other Boost libraries, not client code outside of Boost. Client code
> typically sees the breaking change after the release, or at best after the
> beta, not before. So breaking client code does not in general impede the
> release process, whereas breaking other Boost libraries (or otherwise
> introducing regressions into the testing and release procedures) most
> certainly can and does.
>
> But a permission from a release manager cannot hurt if you decide to
> pursue this.
>
>
I did a short test where I use the old mt19937 based generator in a loop to
make 1 million uuids
and then I compared that to a new random_device implementation (both under
basic_random_generator using the default constructor), and the results
surprised me.  On
a Windows 10 laptop with a Xeon processor, using Visual Studio 2017 and
building release:

Benchmark Times reusing a generator (1M loops):
  old implementation:
 0.021822s wall, 0.031250s user + 0.000000s system = 0.031250s CPU (143.2%)
  new implementation:
 0.373160s wall, 0.375000s user + 0.000000s system = 0.375000s CPU (100.5%)

Benchmark Times using a new generator for each uuid (10K loops):
  old implementation:
 1.168479s wall, 1.171875s user + 0.000000s system = 1.171875s CPU (100.3%)
  new implementation:
 0.010272s wall, 0.015625s user + 0.000000s system = 0.015625s CPU (152.1%)

Asking the OS for 16 bytes of entropy at a time (Wincrypt, 1M loops):
 0.244231s wall, 0.234375s user + 0.000000s system = 0.234375s CPU (96.0%)

So the initialization of the mersenne twister is expensive, so if you do it
once and then
reuse it, it is much more efficient than going to Wincrypt.   If you make a
new generator for
each uuid, the opposite is true.  I expect folks will do the former, but
I've seen the latter
in some projects.

It looks like it is very expensive to get entropy through Wincrypt (and
BCrypt), whereas using
it only to seed a PRNG and then reusing the PRNG it is much more efficient
through the existing
code paths.  Therefore keeping the default type at mt19937 and using this
newer header-only
random_device will maintain performance and compatibility, but also fix the
problems in seed_rng
like how it would ignore errors, and will add UWP support.

Even asking Wincrypt for 16 bytes of entropy at a time is not efficient, so
rewriting the
implementation to be less generic won't improve performance.

So in short, I'll plan to keep the same implementation of random_device but
put in a new seed
mechanism for the PseudoRandomNumberGenerator.  I also fixed up the
template specialization
that was in seed_rng so that any UniformRandomNumberGenerator can be used.
Just
not sure this will go into 1.66.0 at this point... might be too big of a
structural change internally.

- Jim

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
James E. King, III wrote:

> Benchmark Times reusing a generator (1M loops):
>   old implementation:
>  0.021822s wall, 0.031250s user + 0.000000s system = 0.031250s CPU
> (143.2%)
>   new implementation:
>  0.373160s wall, 0.375000s user + 0.000000s system = 0.375000s CPU
> (100.5%)
>
> Benchmark Times using a new generator for each uuid (10K loops):
>   old implementation:
>  1.168479s wall, 1.171875s user + 0.000000s system = 1.171875s CPU
> (100.3%)
>   new implementation:
>  0.010272s wall, 0.015625s user + 0.000000s system = 0.015625s CPU
> (152.1%)

These results look odd to me. What code exactly is being tested?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
On Sat, Nov 4, 2017 at 10:59 PM, Peter Dimov via Boost <
[hidden email]> wrote:

> James E. King, III wrote:
>
> Benchmark Times reusing a generator (1M loops):
>>   old implementation:
>>  0.021822s wall, 0.031250s user + 0.000000s system = 0.031250s CPU
>> (143.2%)
>>   new implementation:
>>  0.373160s wall, 0.375000s user + 0.000000s system = 0.375000s CPU
>> (100.5%)
>>
>> Benchmark Times using a new generator for each uuid (10K loops):
>>   old implementation:
>>  1.168479s wall, 1.171875s user + 0.000000s system = 1.171875s CPU
>> (100.3%)
>>   new implementation:
>>  0.010272s wall, 0.015625s user + 0.000000s system = 0.015625s CPU
>> (152.1%)
>>
>
> These results look odd to me. What code exactly is being tested?
>
>
I went over it a number of times to be sure, and stepped through with the
debugger
to make sure it was actually going into the right code paths.  I'll have to
recreate the
test as a separate benchmark that I can submit and build as part of the
project.

I was using boost::timer::auto_cpu_timer to do the timing.

The "old implementation" is the current boost 1.65.1
uuids::random_generator.
The "new implementation" would be the header-only random_device from my
Boost.Random PR
wrapped inside uuids::basic_random_generator, so there would be no seed()
method and it would
acquire 4 bytes of entropy at a time.  That's why I wrote a separate test
to acquire 16 bytes of
entropy at a time, and that too was slower than reusing mt19937 for the
same number of loops
to generate uuids.

When I have a PR to reference, I'll post it.

- Jim

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost.Uuid and header-only support

Boost - Dev mailing list
James E. King, III wrote:

> On Sat, Nov 4, 2017 at 10:59 PM, Peter Dimov via Boost
> <[hidden email]> wrote:
>
> > James E. King, III wrote:
> >
> > Benchmark Times reusing a generator (1M loops):
> >>   old implementation:
> >>  0.021822s wall, 0.031250s user + 0.000000s system = 0.031250s CPU
> >> (143.2%)
> >>   new implementation:
> >>  0.373160s wall, 0.375000s user + 0.000000s system = 0.375000s CPU
> >> (100.5%)
> >>
> >> Benchmark Times using a new generator for each uuid (10K loops):
> >>   old implementation:
> >>  1.168479s wall, 1.171875s user + 0.000000s system = 1.171875s CPU
> >> (100.3%)
> >>   new implementation:
> >>  0.010272s wall, 0.015625s user + 0.000000s system = 0.015625s CPU
> >> (152.1%)
> >>
> >
> > These results look odd to me. What code exactly is being tested?
> >
> >
> I went over it a number of times to be sure, and stepped through with the
> debugger to make sure it was actually going into the right code paths.
> I'll have to recreate the test as a separate benchmark that I can submit
> and build as part of the project.

I was confused by the first set using 1M loops, and the second one using
only 10K. Didn't catch that at first, so the results didn't make any sense.
Need to multiply the second test by 100 for the two to be comparable.

Have you tried RtlGenRandom by any chance?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
123