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

Add a simple method to run a block every X time #4349

Closed
wants to merge 4 commits into from

Conversation

meew0
Copy link

@meew0 meew0 commented Apr 29, 2017

I haven't received a response to #4255 yet, so I'm not sure if this method actually belongs in the standard library, but this is how I'd implement it, and I'd certainly use this method a lot in my code. Closes #4255.

@meew0 meew0 changed the title Add a simple method to run a block every X seconds Add a simple method to run a block every X time Apr 29, 2017
@konovod
Copy link
Contributor

konovod commented Apr 29, 2017

What should it do if a task takes more time then given span? In your implementation it will call sleep with negative time, i think it should do something like Fiber.yield or sleep(0) (to prevent monopolization). If sleep with negative time works the same way, no problems.

@meew0
Copy link
Author

meew0 commented Apr 29, 2017

That's a good point, I hadn't thought of that. sleep with negative time causes an error so I added a specific check.

@kostya
Copy link
Contributor

kostya commented Apr 29, 2017

why not just run yield in spawn? and not calculate negative?

counter = 0

spawn do
every(10.milliseconds) do |i|
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest at least tripling all the time spans to even have a chance of this test passing consistently.

Copy link
Member

Choose a reason for hiding this comment

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

Why? Even 1ms is aeons at computer speeds. if it turns out to be a problem it can probably be increased.

Copy link
Author

Choose a reason for hiding this comment

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

It appears to pass very consistently on my system (not a single random failure in 1000 test runs). Is this a problem anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

I was wrong, I tried this on my system and it does fail on relatively minor idle jitter events.

@hugoabonizio
Copy link
Contributor

FWIW, we can keep this feature in a separete lib https://github.com/hugoabonizio/schedule.cr

@asterite
Copy link
Member

I don't think this belongs in the std.

@asterite asterite closed this Sep 29, 2017
@z64 z64 mentioned this pull request Apr 12, 2020
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.

None yet

7 participants