xpra icon
Bug tracker and wiki

Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#24 closed enhancement (fixed)

network read loop is highly inefficient

Reported by: Antoine Martin Owned by: Antoine Martin
Priority: major Milestone: 0.0.7.x
Component: client Version: 0.0.7.26
Keywords: Cc:

Description

Whenever we read a bit of data from the socket (sometimes as small as just one character!) we schedule the main loop to call _handle_read. When it actually fires there may only be (sometimes less than) one real packet waiting there, yet it will fire as many times as the socket read loop originally fired.
We want to ensure we don't schedule it again if it is already pending, this should save a lot of context switches and reduce load significantly. Using an atomic loop counter should probably be enough to achieve this.

Change History (2)

comment:1 Changed 9 years ago by Antoine Martin

Resolution: fixed
Status: newclosed

r167 improves this by not scheduling unless necessary (ie: not if the _handle_read has already been scheduled and not run yet).
This uses optimistic locking and may still fire unnecessarily in rare cases (as before - so no harm done).
Python lacks atomic counters (what an omission!) so I added a simple CachedCounter class.

The end result is better as we avoid many thread context switches (note how the Queueing main thread call happens once rather than for every read), however the data is still passed to the _handle_read in dribs and drabs:

read thread: waiting for data to arrive
main thread: woken to handle read data
read thread: got data '0'
Queueing main thread call to <bound method Protocol._handle_read of <xpra.protocol.Protocol object at 0x140fe50>>
read thread: waiting for data to arrive
read thread: got data '0ee'
read thread: waiting for data to arrive
read thread: got data '4:j'
read thread: waiting for data to arrive
read thread: got data 'peg'
read thread: waiting for data to arrive
read thread: got data 'i'
read thread: waiting for data to arrive
read thread: got data '4'
read thread: waiting for data to arrive
read thread: got data '0ee'
read thread: waiting for data to arrive
read thread: got data 'e'
read thread: waiting for data to arrive
main thread: found read data '8'
main thread: found read data '0'
main thread: found read data '0ee'
main thread: found read data '4:j'
main thread: found read data 'peg'
main thread: found read data 'i'
main thread: found read data '4'
main thread: found read data '0ee'
main thread: found read data 'e'
got ['hello', {'desktop_size': [480, 800], '__prerelease_version': '0.0.7.26', 'jpeg': 40}]

Which means that each of these character(s) end up as a single element on the read queue! Waste of space.
The easiest way to change that would be to change the protocol layer to have clear delimiters for data so that it could be buffered until ready for consumption (this is why WinSwitch uses a line protocol where each command is separated by a carriage return...). This would mean breaking compatibility with existing clients though... so unlikely to happen at present unless this can be enabled selectively for clients that support it via the "capabilities" dict.
Anyway, most packets are actually much bigger (ie: damage requests) so this should not be a huge problem. Except maybe for pointer motion events?
Closing as this is probably enough for now.

comment:2 Changed 8 years ago by Antoine Martin

Version: 0.0.7.26
Note: See TracTickets for help on using tickets.