[PATCH weston] xwm: flush xcb connection only when we processed some event

[Derek Foreman]

On 14/05/15 12:12 PM, Derek Foreman wrote:
> On 14/05/15 05:04 AM, Marek Chalupa wrote:
>> And also write out a warning when we got some event that
>> we cannot handle.
>>
>> Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
>> ---
>>  xwayland/window-manager.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
>> index 039f0cd..b1fd12d 100644
>> --- a/xwayland/window-manager.c
>> +++ b/xwayland/window-manager.c
>> @@ -1930,13 +1930,17 @@ weston_wm_handle_event(int fd, uint32_t mask, void *data)
>>  		case XCB_CLIENT_MESSAGE:
>>  			weston_wm_handle_client_message(wm, event);
>>  			break;
>> +		default:
>> +			fprintf(stderr, "WARNING: xwm unhandled event: %p %d\n",
>> +				event, event ? EVENT_TYPE(event) :-1);
> 
> I'm surprised at how often this actually triggers - maybe we don't want
> to flood up the logs with this?
> 
>>  		}
>>  
>>  		free(event);
>>  		count++;
>>  	}
> 
> Ok, wait, why are we even getting here without any data for
> weston_wm_handle_event() to read?
> 
> Oh, because weston_wm_create() calls wl_event_source_check() on that
> event source, so weston_wm_handle_event() is called any time anything in
> the event loop triggers.  And it gets called twice if its fd was what
> actually caused epoll_wait() to complete...
> 
> Is there a reason we need to do that?  Won't epoll() just notice when
> there's something available on that fd?
> 
>> -	xcb_flush(wm->conn);
>> +	if (count != 0)
>> +		xcb_flush(wm->conn);
>>  
>>  	return count;
>>  }
>>
> 

Still in patchwork...  did some archeology this time.

wayland commit 589e581f explains why xwayland must use
wl_event_source_check()

I don't like the fprintf bit because it does happen quite frequently.
I'm not sure the information is useful.  If we really want it, we
probably need to throttle it so we don't flood up the logs.  I guess
that bit should really be in its own patch anyway...

I think the conditional xcb_flush() has merit, since we end up doing an
extra flush pretty much every single time we process events.

So, for just the last half,

Reviewed-by: Derek Foreman <derekf at osg.samsung.com>