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 srand with hostname and PID #738

Merged
merged 1 commit into from
Sep 15, 2024
Merged

Conversation

FalacerSelene
Copy link
Contributor

We're running SIPp in a container scaleset, and ran into an issue where two instances started in the same second and continually picked the same random numbers as each other, which meant that InputFileRandomOrder actually had the instances continually 'randomly' picking the same entries.

To avoid this, this change adds a bit more noise to the start-of-day call to srand(), mixing in the PID and the hostname, which would avoid two instances using the same seed.

I'd welcome an alternative and better way to do this!

@orgads
Copy link
Contributor

orgads commented Aug 1, 2024

Claude suggested this:

unsigned int generate_seed() {
    unsigned int seed = (unsigned int)time(NULL);
    seed ^= (unsigned int)clock();
    seed ^= (unsigned int)getpid();
    
    struct timespec ts;
    if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
        seed ^= (unsigned int)ts.tv_nsec;
    }
    
    return seed;
}

WDYS?

@FalacerSelene
Copy link
Contributor Author

Claude suggested this:

unsigned int generate_seed() {
    unsigned int seed = (unsigned int)time(NULL);
    seed ^= (unsigned int)clock();
    seed ^= (unsigned int)getpid();
    
    struct timespec ts;
    if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
        seed ^= (unsigned int)ts.tv_nsec;
    }
    
    return seed;
}

WDYS?

I don't think we gain from mixing 3 different time-based values (time(), clock() and clock_gettime()) - that still gives only time-based uniqueness. So I'd probably just take clock_gettime()'s nanoseconds and forget about time() and clock() in that case, because it's incredibly unlikely that two processes call that in the same ns.

However, clock_getres() might indicate the resolution isn't really to the nanosecond. Since we don't know the resolution of the clock across systems that sipp might run on, I thought it was better to grab other sources of uniqueness (PID and hostname).

But a suggestion like this would almost definitely solve my problem too :)

@orgads
Copy link
Contributor

orgads commented Aug 16, 2024

Claude suggested this:

unsigned int generate_seed() {
    unsigned int seed = (unsigned int)time(NULL);
    seed ^= (unsigned int)clock();
    seed ^= (unsigned int)getpid();
    
    struct timespec ts;
    if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
        seed ^= (unsigned int)ts.tv_nsec;
    }
    
    return seed;
}

WDYS?

I don't think we gain from mixing 3 different time-based values (time(), clock() and clock_gettime()) - that still gives only time-based uniqueness. So I'd probably just take clock_gettime()'s nanoseconds and forget about time() and clock() in that case, because it's incredibly unlikely that two processes call that in the same ns.

However, clock_getres() might indicate the resolution isn't really to the nanosecond. Since we don't know the resolution of the clock across systems that sipp might run on, I thought it was better to grab other sources of uniqueness (PID and hostname).

But a suggestion like this would almost definitely solve my problem too :)

Sounds good. So can you implement it?

@FalacerSelene FalacerSelene force-pushed the master branch 2 times, most recently from d350b32 to bfe53a2 Compare August 19, 2024 19:46
@FalacerSelene
Copy link
Contributor Author

Sounds good. So can you implement it?

Yep, updated to XOR with the nanosecond value too :)

@orgads
Copy link
Contributor

orgads commented Sep 8, 2024

Any idea why the test fails?

@orgads
Copy link
Contributor

orgads commented Sep 15, 2024

@jeannotlanglois ?

@orgads
Copy link
Contributor

orgads commented Sep 15, 2024

Rebased and it passed. Merging.

Thanks for your persistence @FalacerSelene!

@orgads orgads merged commit 5e17500 into SIPp:master Sep 15, 2024
8 checks passed
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