Remove code relating to parallel execution from PeriodicalExecuter.

[arbales]

In PeriodicalExcuter, removed unnecessary currentlyExecuting ivar and other code relating to parallel execution.

Since #onTimerEvent contained no logic after this change, made setInterval call (a bound) #execute and renamed #registerCallback to #start so that the timer can be restarted at will. This behavior already existed in the previous (undocumented) function.

Thanks!

[arbales]

Didn't seem like this was performing any function that the previous test wasn't already doing, given that currentlyExecuting was removed.

[arbales]

(ping)

[arbales]

Mind merging or closing?

[cecilphillip]

I'm guessing they've given up over here

[arbales]

That's sad. Is there a a canonical fork? I don't use the library anymore, but I think lots do.

Sent from my thumbs.

On Mar 1, 2012, at 7:15 PM, Cecil Phillipreply@reply.github.com wrote:

I'm guessing they've given up over here

---

Reply to this email directly or view it on GitHub:
#23 (comment)

[savetheclocktower]

Sorry — we dropped the ball on pull requests for a while.

Can I ask why you think the code relating to parallel execution is unnecessary? I'm confused. It looks quite necessary to me if the point is to prevent an accumulation of callbacks in the case where your callback routinely takes longer than the interval. If your point is that JavaScript is single-threaded and thus nothing actually executes in "parallel," then I'll concede that it's a poor choice of words.

Just want to make sure I'm not missing something here.

[savetheclocktower]

HAHAHA DISREGARD THAT (now I see your point)

Just shows how little this code has been touched over the years. Yeah, it doesn't prevent callback accumulation, and it's quite silly of us to have pretended that it did.

So I have four options, as I see it:

1. Change the documentation to match existing behavior, but otherwise do nothing.
2. Change the documentation to match existing behavior. Also, admit defeat on the issue and apply this patch, thereby conceding that PeriodicalExecuter is nothing more than a thin wrapper around setInterval.
3. Change it to call setTimeout at the end of the callback execution, thus guaranteeing at least frequency seconds between calls (a somewhat-significant change in behavior).
4. Change the behavior to match existing documentation. In other words, adopt an actual strategy for preventing callback accumulation. For instance, onTimerEvent could keep track of how long it's been since the last time it was called. Like, if we've got a frequency of one second, and onTimerEvent sees that it's been more than two seconds since the last time it was called, it could just return early, knowing that setInterval has scheduled another call to onTimerEvent that ought to run in no time flat.

I am leaning towards option #4 because we can't get rid of PeriodicalExecuter (backward compatibility), and as long as we're stuck with it I think it should serve some purpose beyond setInterval.