Code review comment for lp:~vanvugt/compiz-core/fix-timer-infinite-loop

On Fri, Nov 18, 2011 at 7:01 PM, Sam Spilsbury <email address hidden> wrote:

>
>
> On Sat, Nov 12, 2011 at 12:14 PM, Daniel van Vugt <email address hidden>wrote:
>
>> A test case without X events happening is a bit tricky. But I think
>> something like creating 2 timers:
>> (a) interval 0
>> (b) interval 500
>> And then waiting 2000ms, verifying that (b) has been called >= 2 times,
>> should work. With the bug, (a) will always stay in front of (b) so (b) is
>> never called.
>>
>> I'll look at doing this soon.
>>
>> On a related note, is there a good reason why CompTimer and the
>> associated classes are so complex? I tried rewriting CompTimer with
>> g_timeout_add_full, and deleted the associated timeoutsource/handler
>> classes, and found it performed much better (due to better pre-emption -
>> other events can be handled between timer callbacks) and was much smaller...
>>
>
> Yes, ordering becomes a problem when timers change their timeout times
> while the timer is already "started" (the composite plugin was doing this
> and it caused redraw issues).
>

Well, to clarify on this point - GLib doesn't have a concept of "changing"
a timeout time while a timer is already running. You can only remove a
timer and re-add it with the new timeout time. If you had 4 timers with the
same timeout time (eg, zero or something) then this would place the newly
added timer at the back of the queue.

>
>
>> --
>>
>> https://code.launchpad.net/~vanvugt/compiz-core/fix-timer-infinite-loop/+merge/81944
>> You are reviewing the proposed merge of
>> lp:~vanvugt/compiz-core/fix-timer-infinite-loop into lp:compiz-core.
>>
>
>
>
> --
> Sam Spilsbury
>

--
Sam Spilsbury