[mythtv] Re: [PATCH] Increase block size

[avalanche at beyondmonkey.com]

> Bruce Markey wrote:
> >Mark Frey wrote:
> >I the existing code, there is the concept of requestedbytes that
> >sends REQUEST_BLOCKs ahead of time so the pipeline is always
> >moving. It appears that in RemoteFile::Read a request is sent,
> >it waits to read that data then the network is dormant until
> >the next Read. Is this correct? It may be okay but it seems
> >less efficient.
> 
> That's correct, but I would guess that this is a non-issue. Here's my
> reasoning: The old code essentially tried to keep the requested bytes near
> 128000 bytes (it actually wound up requesting about 64000 bytes every time
> in the steady-state case). It then waited until it got a response from the
> backend and 64000 were available and exited from RingBuffer::RequestBlock()
> until the next time through. The magic number here was 130000 which was the
> size of the underliying socket's send/recieve buffer. However, the way the
> actual request worked was as follows:
> 
> >Part of av's QSocketDevice patch is that he needed to stay
> >below a limit of about 130k somewhere. However, I see you
> >are sending and receiving 256000 blocks. How is it that this
> >isn't a problem?
> 
> The problem with the old code was that the frontend thread waited for a
> response from the backend on the control socket. This response wasn't sent
> until the backend had sent all the requested bytes, therefore, the number of
> bytes requested had to be less than the buffer size. If not a deadlock
> occured: the frontend waiting for a response, while the backend waits for
> the frontend to clear out the socket so it can send more data. With the
> patch the frontend sends the request, and then starts reading from the data
> socket without waiting for a response. Once it's received all the data it
> requested, or times out, or sees that a response is in the control socket,
> it completes (this is not exactly what happens, there is a slight wrinkle,
> but this is the basic idea). In this way we don't care what size the
> underlying socket buffer is.

The old QSocket code originally requested a block and waited till it was 
available, that worked well since QSocket automatically cleared the socket 
buffer to an internal buffer, making bigger blocks possible. With the QSD 
changes the double buffer was lost, that's where the 64k/130k limits came 
from. The new patch ads the double buffered logic back, so it fits in 
nicely.

> >Before the QSocketDevice patch the block size was variable
> >based on estbitrate. On low bitrate data, waiting to complete
> >large blocks can add unnecessary latency. This may be easy to
> >fix by changing using the block size calculations from a few
> >versions back.
> 
> I would agree here. I think there certainly is room for improvement on
> choosing the block size based on whatever heuristic criteria (bitrate,
> connection speed, whatever). Again, as a first step I wanted to break the
> requirement that request size <= 130000. In profiling the code I found that
> the overhead of making the request to the backend was high, and so I wanted
> to amortize this cost over more bytes == larger requested block size.

I attached the complete patch with a few changes:
Added the old scaling code, default block size is now 128k. Scaling could 
start lower than 128k but I don't think it's going to make much difference 
since everything up to 128k is all one go. I also added some stuff that
should stop fragmenting if size > socket buffer size, 256k fits to 2x128k 
instead of 3xX. Did some other tiny adjustments (80 columns ;) etc, I think 
it should be ready for cvs now, although more testing wouldn't hurt.


av
-------------- next part --------------
A non-text attachment was scrubbed...
Name: blocksize#2.diff
Type: application/octet-stream
Size: 20510 bytes
Desc: not available
Url : http://mythtv.org/pipermail/mythtv-dev/attachments/20031223/31869b81/blocksize2-0001.obj