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

initialize random udp package id using nanosecond timestamp #1270

Merged
merged 2 commits into from
Nov 24, 2023

Conversation

rex-schilasky
Copy link
Contributor

Description

Random UDP package id now uses timestamp to start with a unique number for every process.

Cherry-pick to

  • 5.11 (old stable)
  • 5.12 (current stable)

unsigned long xorshf96(unsigned long& x, unsigned long& y, unsigned long& z) // period 2^96-1
{
const std::lock_guard<std::mutex> lock(xorshf96_mtx);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you have a mutex here? what do you want to protect? This function has no global variables which it might access, hence it is reentrant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that is right :)

@rex-schilasky rex-schilasky merged commit 743384c into master Nov 24, 2023
14 checks passed
@rex-schilasky rex-schilasky deleted the bugfix/initialize-udp-header-id branch November 24, 2023 14:56
static std::mutex xorshf96_mtx;
const std::lock_guard<std::mutex> lock(xorshf96_mtx);

static unsigned long x = static_cast<unsigned long>(std::chrono::duration_cast<std::chrono::nanoseconds>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rex-schilasky I was wondering if std::random would be a bit more appropriate here, to indicate we need a random number to start with. If you started two processes at the exact same time, would they get the same numbers this way? Though still unlikely.

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

Successfully merging this pull request may close these issues.

2 participants