Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UUID: Use the non-cryptographic variant of the boost::uuid #9832

Merged
merged 2 commits into from
Dec 11, 2020

Conversation

rgacogne
Copy link
Member

@rgacogne rgacogne commented Dec 8, 2020

Short description

Since Boost 1.67.0 the default UUID generator is cryptographically strong, which is neat but quite slower. Since we don't need that, just use the fastest version.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@rgacogne
Copy link
Member Author

rgacogne commented Dec 8, 2020

Whoops, looks like I cleaned my code a bit too much after testing since it doesn't compile anymore, fixing that!

Since Boost 1.67.0 the default UUID generator is cryptographically
strong, which is neat but quite slower. Since we don't need that,
just use the fastest version.
@omoerbeek
Copy link
Member

omoerbeek commented Dec 8, 2020

We have to make sure a non-seeded mt19937 does not always start in the same state, and thus generate the same uuids for different runs. I'll check this soon.

@rgacogne
Copy link
Member Author

rgacogne commented Dec 8, 2020

That's a very good point! From the documentation in the uuid class I did not expect we needed to seed it, but the documentation in the random class makes me think it can very well be. Now I wonder if we have been doing it wrong in the past..

@zeha
Copy link
Collaborator

zeha commented Dec 8, 2020

The old boost implementations (which used MT) seem to seed it at construct time.

@rgacogne
Copy link
Member Author

rgacogne commented Dec 8, 2020

The basic_random_generator default constructor has a comment pretending it will be correctly seeded, but let's check ourselves:

    // default constructor creates the random number generator and                                                                                                                                                   
    // if the UniformRandomNumberGenerator is a PseudoRandomNumberGenerator                                                                                                                                          
    // then it gets seeded by a random_provider.   

@omoerbeek
Copy link
Member

omoerbeek commented Dec 9, 2020

Test on OpenBSD (boost 1.70) shows the MT is seeded. Original code:

'UUIDGenTest' 0.21 seconds: 730515.1 runs/s, 1.37 usec/run

New code:

'UUIDGenTest' 0.10 seconds: 9190128.8 runs/s, 0.11 usec/run

I'll push the speedtest to this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants