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

Use unsynchronized random number generator. #229

Merged
merged 1 commit into from
Sep 18, 2017

Conversation

cardbot
Copy link

@cardbot cardbot commented Sep 16, 2017

The skiplist benchmark was using the global random number generator,
which is guarded by a mutex. This causes contention on multi-core
machines and skews the benchmark results in a material way. Instead,
use a separate random number generator per benchmark goroutine.
Update the README results.


This change is Reviewable

The skiplist benchmark was using the global random number generator,
which is guarded by a mutex. This causes contention on multi-core
machines and skews the benchmark results in a material way. Instead,
use a separate random number generator per benchmark goroutine.
Update the README results.
@CLAassistant
Copy link

CLAassistant commented Sep 16, 2017

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@manishrjain
Copy link
Contributor

LGTM. Thanks for the change. Once you sign the CLA, I'll merge it.


Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@cardbot
Copy link
Author

cardbot commented Sep 16, 2017

@manishrjain, I signed the CLA, but it's not updating, even when I click the "Let us recheck it" link. So maybe I need to give it some time to refresh. Also, the CI tests failed in a way that looks unrelated to my change (flaky test?). I'm not sure how to re-run them.

screen shot 2017-09-16 at 10 18 16 pm

@manishrjain
Copy link
Contributor

:lgtm: Yeah, I can see that you signed. Not sure why CLA isn't updating.


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@manishrjain manishrjain merged commit 231082b into dgraph-io:master Sep 18, 2017
srh pushed a commit that referenced this pull request Sep 21, 2017
The skiplist benchmark was using the global random number generator,
which is guarded by a mutex. This causes contention on multi-core
machines and skews the benchmark results in a material way. Instead,
use a separate random number generator per benchmark goroutine.
Update the README results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants