Re: scalable event scheduling

Alex Rousskov <rousskov_at_measurement-factory.com> writes:
> On 01/30/2013 09:30 AM, Rainer Weikusat wrote:
>> Alex Rousskov <rousskov_at_measurement-factory.com> writes:
>>> On 01/28/2013 03:29 PM, Rainer Weikusat wrote:
>>>> so easy that even using the STL implementation wouldn't be worth the
>>>> effort
>>>
>>> Can you clarify why we should not just use one of the STL queues
>>> instead?
>>
>> I agree with your opinion that "the STL doesn't contain an 'obviously
>> good' choice for a priority queue implementation in the given
>> context".
>
> I am glad we agree on this, but I think that STL must be seriously
> considered before we add something custom.

It is not suitable in this case and reading through the documentation
should also show that as the comparable 'STL thing' lacks a 'delete
any object' interface. I'm also no more convinced of the merits of the
STL approach than the people who wrote (or didn't change) all the
other existing code which could theoretically use it, up to the
linked-list based events.cc itself.

[...]

>>>> the event scheduler will store a value at the
>>>> pointed-to location which can be passed to eventDelete to cancel a pending
>>>> event 'safely' in logarithmic time, meaning, even if the event was
>>>> possibly already put onto the async queue.
>
>> The purpose of the eventDelete routine is not to cancel a call the
>> event scheduler already put onto the async call queue but to prevent
>> such a call from being put onto this queue if this is still
>> possible. And I didn't intend change that.
>
> Noted, but I am still missing something: Why add a new event tag API if
> the existing eventDelete() code can already cancel events outside the
> async queue?

I already explained this in my first mail on this topic and I will
gladly answer (reasonable) questions based on what I wrote there.

[...]

> I do not think such "occasionally working" cancellation is a good idea
> though.

There are two different queueing subsystems involved here and because
of this, users of the code will have to deal with both. This may be
regarded as unfortunate design choice but in any case, it wasn't my
choice[*].

[*] In order to get stuff done, one has to start with something
reasonably simple that it can be accomplished relatively easily on its
own and deal with the remaining deficits of 'the universe in the
pitiful state it generally happens to be in' afterwards. At least, it
will be a somewhat less pitiful state then.

[...]

> I also think that the need for explicit destruction of the event "tag"
> by the caller is a serious downside because it increases the risk of
> memory leaks.

No more serious than all the existing situations where this is already
necessary, including kernel objects like file descriptors [*].

> FWIW, here is how I think the event cancellation should be supported
> instead:
>
> 1. Another form of eventAdd() should be added to accept caller's async
> call. New code, especially async jobs, should use that form (jobs will
> and avoid call wrappers that way). Old code that wants to cancel events
> should use this new form as well.

This would imply keeping cancelled events on the event scheduler queue
until they expire 'on their own' because the metainformation necessary
to remove them efficiently from this queue is not available in this
way. That's another thing I used to do in the past but the amount of
code which doesn't need to be written because of this (essentially, a
single if-statement) doesn't warrant the runtime overhead of not
providing this functionality, especially in the case of a 'universal'
timer subsystem which is both useful for tasks which are usually
expected to be executed and tasks which usually won't run (like
timeouts).

A better idea would be to add an interface which takes a call instead
of the function pointer + argument pointer as argument so that all
options would be available to be used whenever their use happens to
make sense (an even better idea would be to integrate these two
separate things in some 'closure-like' object and to relieve users
from the burden of worrying about two pointers instead of one but
that's sort of a digression).

[...]

> The current event scheduler already creates an async call object for
> nearly all events anyway, so this does not add CPU overhead. The only
> overhead is that we will consume the same [small] amount of RAM
> sooner.

It actually does this for all events. This kind of double-indirection
is what makes getting rid of an even which is possibly still on the
event-scheduler queue so difficult (as I wrote in my reply to
Amos). Unless there's (again) something I'm missing, running event
scheduler events directly from the EventScheduler::checkEvents routine
should be considered as 'general case'. 'Running async calls' could be
provided on top of that using a suitable, general 'event running
routine'.
Received on Thu Jan 31 2013 - 02:13:25 MST