Re: trunk r116456: Improve dbus performance on synchronous calls

Re: trunk r116456: Improve dbus performance on synchronous calls

[Michael Albinus]

Daniel Colascione <dan.colascione@gmail.com> writes:

>      (with-timeout ((if timeout (/ timeout 1000.0) 25))
>        (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
> -	(let ((event (let ((inhibit-redisplay t) unread-command-events)
> -		       (read-event nil nil 0.1))))
> -	  (when (and event (not (ignore-errors (dbus-check-event event))))
> -	    (setq unread-command-events
> -		  (append unread-command-events (list event)))))))
> +        (let ((event (let ((inhibit-redisplay t) unread-command-events)
> +		       (read-event nil nil check-interval))))
> +          (when event
> +            (push event unread-command-events))
> +          (when (< check-interval 1)
> +            (setf check-interval (* check-interval 1.05))))))

Why `push'? It adds the event on top of `unread-command-events',
changing the event order when there are already events in that list.

Best regards, Michael.

Re: trunk r116456: Improve dbus performance on synchronous calls

[Daniel Colascione]

On 02/17/2014 08:06 AM, Michael Albinus wrote:
> Daniel Colascione <dan.colascione@gmail.com> writes:
>
>>       (with-timeout ((if timeout (/ timeout 1000.0) 25))
>>         (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
>> -	(let ((event (let ((inhibit-redisplay t) unread-command-events)
>> -		       (read-event nil nil 0.1))))
>> -	  (when (and event (not (ignore-errors (dbus-check-event event))))
>> -	    (setq unread-command-events
>> -		  (append unread-command-events (list event)))))))
>> +        (let ((event (let ((inhibit-redisplay t) unread-command-events)
>> +		       (read-event nil nil check-interval))))
>> +          (when event
>> +            (push event unread-command-events))
>> +          (when (< check-interval 1)
>> +            (setf check-interval (* check-interval 1.05))))))
>
> Why `push'? It adds the event on top of `unread-command-events',
> changing the event order when there are already events in that list.

Hrm. I was getting it from sit-for:

(let ((read (read-event nil t seconds)))
       (or (null read)
	  (progn
	    ;; If last command was a prefix arg, e.g. C-u, push this event onto
	    ;; unread-command-events as (t . EVENT) so it will be added to
	    ;; this-command-keys by read-key-sequence.
	    (if (eq overriding-terminal-local-map universal-argument-map)
		(setq read (cons t read)))
	    (push read unread-command-events)
	    nil)))

Speaking of which, shouldn't the dbus code just look like this?

(with-timeout ((if timeout (/ timeout 1000.0) 25))
       (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
         (sit-for check-interval t)
         (when (< check-interval 1)
           (setf check-interval (* check-interval 1.05)))))

Re: trunk r116456: Improve dbus performance on synchronous calls

[dancol]

> On 02/17/2014 08:06 AM, Michael Albinus wrote:
>> Daniel Colascione <dan.colascione@gmail.com> writes:
>>
>>>       (with-timeout ((if timeout (/ timeout 1000.0) 25))
>>>         (while (eq (gethash key dbus-return-values-table :ignore)
>>> :ignore)
>>> -	(let ((event (let ((inhibit-redisplay t) unread-command-events)
>>> -		       (read-event nil nil 0.1))))
>>> -	  (when (and event (not (ignore-errors (dbus-check-event event))))
>>> -	    (setq unread-command-events
>>> -		  (append unread-command-events (list event)))))))
>>> +        (let ((event (let ((inhibit-redisplay t)
>>> unread-command-events)
>>> +		       (read-event nil nil check-interval))))
>>> +          (when event
>>> +            (push event unread-command-events))
>>> +          (when (< check-interval 1)
>>> +            (setf check-interval (* check-interval 1.05))))))
>>
>> Why `push'? It adds the event on top of `unread-command-events',
>> changing the event order when there are already events in that list.
>
> Hrm. I was getting it from sit-for:

Nope. You're right: fixed. Thanks for pointing that out.

> Speaking of which, shouldn't the dbus code just look like this?

No, that won't work. It's single-shot.

Re: trunk r116456: Improve dbus performance on synchronous calls

[Michael Albinus]

dancol@dancol.org writes:

>>> Why `push'? It adds the event on top of `unread-command-events',
>>> changing the event order when there are already events in that list.
>>
>> Hrm. I was getting it from sit-for:
>
> Nope. You're right: fixed. Thanks for pointing that out.

Maybe `sit-for' is wrong then as well? I haven't checked ever (and
situations, where `unread-command-events' contains more than one event
seem to be rare).

Re: trunk r116456: Improve dbus performance on synchronous calls

[Daniel Colascione]

On 02/17/2014 01:08 PM, Michael Albinus wrote:
> dancol@dancol.org writes:
>
>>>> Why `push'? It adds the event on top of `unread-command-events',
>>>> changing the event order when there are already events in that list.
>>>
>>> Hrm. I was getting it from sit-for:
>>
>> Nope. You're right: fixed. Thanks for pointing that out.
>
> Maybe `sit-for' is wrong then as well? I haven't checked ever (and
> situations, where `unread-command-events' contains more than one event
> seem to be rare).

sit-for is right: it's reading one event, then immediately sticking it 
back on the queue. Push is exactly the right operation there.

Re: trunk r116456: Improve dbus performance on synchronous calls

[Stefan Monnier]

> Maybe `sit-for' is wrong then as well?

No, the `push' is right: if unread-command-events is non-nil at that
point, it means it holds events which `read-event' did not read yet
(and hence come *after* the event we just received).


        Stefan