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 Perdiodic Timer and nextiterationSleepingTime #29

Merged
merged 1 commit into from
Apr 25, 2018

Conversation

DocRoms
Copy link
Contributor

@DocRoms DocRoms commented Apr 17, 2018

Update the documentation for DaemonBundle usage.

@DocRoms DocRoms force-pushed the feature/add-event-loop-documentation branch from c873c05 to 805f0f9 Compare April 17, 2018 13:13
Copy link
Member

@Oliboy50 Oliboy50 left a comment

Choose a reason for hiding this comment

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

👍

README.md Outdated
@@ -41,6 +41,8 @@ m6_web_daemon:

## Write command

This command use the [event-loop component](https://github.com/reactphp/event-loop#usage) that uses [ReactPHP](https://reactphp.org) for execute loops; and other tasks asynchronously.
Copy link
Member

@Oliboy50 Oliboy50 Apr 17, 2018

Choose a reason for hiding this comment

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

that uses ReactPHP for execute loops; and other tasks asynchronously.

=> which ReactPHP uses to run loops and other asynchronous tasks.

(either run or make sounds good to me)

README.md Outdated
@@ -64,6 +66,16 @@ class MyDaemonizedCommand extends DaemonCommand

// Add your own optional callback : called every 10 iterations
$this->addIterationsIntervalCallback(10, [$this, 'executeEveryTenLoops']);

// You can add your own Periodic Timer,
// In this timer will be called every 0.5s.
Copy link
Member

Choose a reason for hiding this comment

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

// here this timer will be called every 0.5s

README.md Outdated

// You can add your own Periodic Timer,
// In this timer will be called every 0.5s.
$daemon = $this;
Copy link
Member

@Oliboy50 Oliboy50 Apr 17, 2018

Choose a reason for hiding this comment

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

i'm pretty sure that PHP already binds $this with the anonymous function parent context

So i'm 90% sure that you can simply do:

$this->loop->addPeriodicTimer(0.5, function ($timer) {
    if ($this->isLastLoop()) {
        $this->loop->cancelTimer($timer);
    }
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is true... Since PHP 5.4 :P See here for details

👍 Thank you

Copy link
Member

Choose a reason for hiding this comment

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

thanks for the link ❤️

README.md Outdated
// This method helps to give back the CPU to the react-loop (here for 1000 $µseconds).
// So you can wait between two iterations if your workers has nothing to do.

// We do not recommand to use sleep() or usleep() anymore.
Copy link
Member

@Oliboy50 Oliboy50 Apr 17, 2018

Choose a reason for hiding this comment

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

recommand
=> recommend

Is there somewhere else in the doc where we did recommend sleep() or usleep() ? Because if it's true, we should also remove these recommendations

anyway, I'm not sure we should say that we recommended it before, it's in the git history forever now, they can look for older versions if they want to see our previous recommendations :)

README.md Outdated
// So you can wait between two iterations if your workers has nothing to do.

// We do not recommand to use sleep() or usleep() anymore.
$this->setNextIterationSleepingTime(1000); // Every milliseconds
Copy link
Member

Choose a reason for hiding this comment

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

you should use another value, this one may be confusing because you put 1000 and say milliseconds

I'd suggest to either explicitly say Every 1000 microseconds

or to use another value instead:

$this->setNextIterationSleepingTime(1000000); // Every second

@DocRoms DocRoms force-pushed the feature/add-event-loop-documentation branch 2 times, most recently from d8fc75c to 3450780 Compare April 17, 2018 15:12
README.md Outdated

usleep(100000);

// This method helps to give back the CPU to the react-loop (here for 1000 $µseconds).
Copy link
Member

Choose a reason for hiding this comment

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

remove (here for 1000 $µseconds)?

README.md Outdated
// This method helps to give back the CPU to the react-loop (here for 1000 $µseconds).
// So you can wait between two iterations if your workers has nothing to do.

$this->setNextIterationSleepingTime(1000000); // Every seconds
Copy link
Member

@Oliboy50 Oliboy50 Apr 17, 2018

Choose a reason for hiding this comment

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

I'm not so sure about that but I think the word after every should not be pluralized, because it means "chaque" in french...

I think 👉 @pabedu 👈 taught me that (you can hit him if it's wrong)

Update comment

Add link to reactPhp

Edit comments and add link

review Fix

Add Note for old PHP version

Typo
@DocRoms DocRoms force-pushed the feature/add-event-loop-documentation branch from 3450780 to af9361d Compare April 25, 2018 15:57
@DocRoms DocRoms merged commit ed5e308 into master Apr 25, 2018
@DocRoms DocRoms deleted the feature/add-event-loop-documentation branch April 25, 2018 16:01
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.

4 participants