Jeremy White a écrit :
Hi Eric,
Thanks for responding.
From an implementation point of view, there's something wrong with it. The time returned from TIME_MMSysTimeCallback (time to wait for next timer to elapse) only takes into account the timers which existed when the function is entered, not the ones which could have been created or deleted while processing the callbacks.
I think this is safe; the wakeup event would be signalled if any events were added while we were in this function, so we should immediately return to that function.
yup. I mixed up the fact that you didn't set the event when destroying a timer, but that shouldn't hurt too much. In the worst case, you'll reenter the loop to recompute the correct value. As rereading the patch, for one-shot timer, you could set the delta_value to INFINITE (since they are destroyed when they expire). In terms of other optimization, you can remove the call to TIME_MMTimeStart in most of the places, except for a timer creation. The other places rely on it to get WINMM_SysTimeMS updated, but since you killed it...
Another point, with your proposal, we could use the fact that the global time no longer needs to be maintained, hence the worker thread can be destroyed when no more timers exist.
I hadn't considered that; I had thought that having the thread sleeping for INFINITE was a pretty nice improvement, but your proposal is arguably even better.
I'd mainly like to get rid of the poor anti-race scheme that's implemented (we can do better than that).
The only case where it would be a poor choice is a situation where events are being created and destroyed continually.
and it's application dependent :-(
But maybe we should start with this patch (and deal with the bugs it introduces <grin>), and I can mark a FIXME to make that improvement in the future.
sure A+