Missing class Executor methods

[janwilmans]

• Executor::Synchronize() was added, this synchronises with any Call() and CallAsync() calls in progress

• Executor::Flush() should wait for all scheduled calls in the queue including CallAt (but not CallEvery)

• Executor::CallAsync should return a ScheduledCall object so it can be cancelled

• ExecutorClient::CancelAll() should cancel all outstanding calls from that client, this requires a design change to store a clientID with the call in the queue, so the call's client can be identified

• ExecutorClient::Flush() should wait for all scheduled calls in the queue, but only from this client, if another client scheduled something afterwards, Flush() can not wait for that.

The use-cases for ExecutorClient::CancelAll and ExecutorClient::Flush are when the Executor is shared by two or more client and one client's lifetime ends, you want to either cancel or flush any pending calls before releasing the client object.

[djeedjay]

CallAsync() is a variant of Call() which returns a future. It is just a function call which should give the guarantee of ultimately being called. If Call cancellation is wanted, a cancellation checkpoint object should be used instead. This is orthogonal to Executor and should not be mixed. If you really, really wanted to, you could CallAt(fn, now()) to fake a CallAsync() which returns a ScheduledCall object.

Synchronising with scheduled calls should not be attempted as this could take really long. Scheduled calls are not guaranteed to run and can be cancelled.

Please provide a use case for each proposal, the proposed changes seem to weaken the guarantees the Executor provides and therefore weaken the structures that can be built on it.

On 12 Dec 2016, at 16:21, Jan Wilmans notifications@github.com wrote:

Executor::Synchronize() was added, this synchronises with any Call() and CallAsync() calls in progress Executor::Flush() should wait for all scheduled calls in the queue including CallAt (but not CallEvery) Executor::CallAsync should return a ScheduledCall object so it can be cancelled ExecutorClient::CancelAll() should cancel all outstanding calls from that client, this requires a design change to store a clientID with the call in the queue, so the call's client can be identified — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/djeedjay/DebugViewPP/issues/251, or mute the thread https://github.com/notifications/unsubscribe-auth/ACz1XCJGk8WwMH8O-00b10GeATfNz40mks5rHWaFgaJpZM4LKqUx.

[janwilmans]

Lets take the use-cases one at the time and start with the current use-case in the ThrottleTest unittest.

class Throttle is a function object that can be used to limit the amount of calls into an object, the use-case is generic, but for Debugview++ it makes sure the ui-thread is not saturated with updates for every single line.

• class Throttle guarantees that at least one call to the destination is always made after the last call to Throttle().
• Throttle guarantees a limited CPS (calls per second) of N cps or lower then N cps if the call itself exceeds the duration of 1/N.

class Throttle builds on class Executor and to test it we make 5 million calls to it with pseudo-random intervals. However to evaluate its results we need to make sure that all calls queued are actually 'done'. Currently there is no way to know this, the new Executor::Synchronize() does not synchronize with CallAt-calls (and that is a good thing).

I agree that the strong guarantee of Call() and CallAsync is important, lets leave that for a future discussion. I propose a Flush() method that does synchronise with all calls in the queue except for CallEvery-calls. The use-cases for Flush() are:

• synchronizing an object with all its outstanding calls, for example before:
  • destroying it
  • evaluating its state

For the unittest-usecase, a Executor::Flush() method would be good enough as it is a single-client of the Executor. However, if there are multiple clients using the same executor, which is more or less the point of having an executor in the first place, it is important that Flush() does not wait for calls of another client, as this would cause unneeded/unwanted synchronization between clients.

This is why I propose an ExecutorClient::Flush() instead of Executor::Flush()

Note 1: I agree that calling ExecutorClient::Flush() should be avoided if possible, as you said, it can take a very long time, however if you call ExecutorClient::Flush(), that is what you want.