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

Added variable latency delay, normal and uniform based #2871

Merged
merged 1 commit into from
Jun 20, 2016

Conversation

rikonor
Copy link
Contributor

@rikonor rikonor commented Jun 18, 2016

Hey,
I saw the todo for random delays and thought I'd give it a go.

License: MIT
Signed-off-by: Or rikonor@gmail.com

func VariableNormal(t, std time.Duration) D {
v := &variableNormal{
std: std,
rng: rand.New(rand.NewSource(int64(time.Now().Nanosecond()))),
Copy link
Member

Choose a reason for hiding this comment

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

Using one random source per one delay instance runs risk of it being used just one time.
Pseudo Random Generators used just one time will not generate uniformly distributed random numbers.

I would recommend using one package wide random generator.

@rikonor
Copy link
Contributor Author

rikonor commented Jun 18, 2016

@Kubuxu, thanks for the feedback.
I added a shared rng instance, and left the possibility of providing your own.
Regarding the locks, two goroutines can wait at the same time because they'll both be able to acquire a read lock.

@Kubuxu
Copy link
Member

Kubuxu commented Jun 18, 2016

Right, I missed that.
Would you mind amending triangular brackets around you events just to make all tests happy.

Apart from that, LGTM, thank you.

@rikonor
Copy link
Contributor Author

rikonor commented Jun 18, 2016

@Kubuxu - yep, fixed that. Thanks!

@Kubuxu
Copy link
Member

Kubuxu commented Jun 18, 2016

LGTM

@Kubuxu Kubuxu added the RFM label Jun 18, 2016
"sync"
"time"
)

var sharedRNG = rand.New(rand.NewSource(int64(time.Now().Nanosecond())))
Copy link
Member

Choose a reason for hiding this comment

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

instead of .Nanosecond() we could use .UnixNano(), saves a cast

@whyrusleeping
Copy link
Member

one comment, then LGTM

@whyrusleeping whyrusleeping added need/author-input Needs input from the original author and removed RFM labels Jun 19, 2016
- Allow providing new delays with your own rng / use shared rng

License: MIT
Signed-off-by: Or Rikon <rikonor@gmail.com>
@rikonor
Copy link
Contributor Author

rikonor commented Jun 19, 2016

@whyrusleeping, using UnixNano now instead of Nanosecond and a cast.

@Kubuxu Kubuxu added RFM and removed need/author-input Needs input from the original author labels Jun 19, 2016
@whyrusleeping
Copy link
Member

cool, LGTM. Thanks!

@whyrusleeping whyrusleeping merged commit 30f2c1f into ipfs:master Jun 20, 2016
@ghost
Copy link

ghost commented Jul 21, 2016

This doesn't seem like it's used anywhere?

@rikonor
Copy link
Contributor Author

rikonor commented Jul 21, 2016

@lgierth I don't believe that it is. I was just looking for something to do and ended up implementing that. Hope that it can help at some point in the future.

@Kubuxu
Copy link
Member

Kubuxu commented Jul 22, 2016

We currently do local network dialups in bursts. This is somewhere we can use it.

@jbenet
Copy link
Member

jbenet commented Jul 22, 2016

Maybe this type of thing could be useful as a libp2p transport?
On Fri, Jul 22, 2016 at 2:09 AM Jakub Sztandera notifications@github.com
wrote:

We currently do local network dialups in bursts. This is somewhere we can
use it.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#2871 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAIcobcnOFAip1QgrY1jJQQAzwozlPDTks5qYBgygaJpZM4I49yt
.

@Kubuxu
Copy link
Member

Kubuxu commented Jul 22, 2016

We might move it to utility package to use it in libp2p to not send dials in bursts, it isn't huge issues right now.

@ghost
Copy link

ghost commented Jul 22, 2016

@jbenet aren't you having a day off? Go out and play :)

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

Successfully merging this pull request may close these issues.

4 participants