Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#11252 closed Patch - Feature (fixed)

Reduce DeviceReadBuffer thread wakeup

Reported by: Rune Petersen <rune@…> Owned by: jpoet
Priority: minor Milestone: 0.27
Component: MythTV - General Version: 0.26-fixes
Severity: medium Keywords:
Cc: Ticket locked: no


This patch affects 2 threads:

DeviceReadBuffer? reader thread:

The reader thread will continually poll and read from the device when data is available. Depending on the driver this will result in the thread will wake up and read() 700+ times a second with a reads of ~2KB (DVB HD content).

The patch calls usleep(1000) if a read is less than 'dev_read_size/2'

This reduces the thread to ~200 read() calls a second with reads of 7-8KB (DVB HD content).

The (DVB/ASI)StreamHandler? thread:

The thread will continually call DeviceReadBuffer::Read() and then process the content. Currently DeviceReadBuffer::Read() will return if more than 4*188 bytes is available in the buffer (timeout 500ms.) This results in DeviceReadBuffer::Read() being called 700+ times a second with a reads of ~2KB (DVB HD content).

The patch changes DeviceReadBuffer::Read() to wait for more data (~24KB) and decrease the timeout to 20 ms. The timeout is now responsible for controlling the maximum latency of the data stream.

This reduces the thread to ~70 DeviceReadBuffer::Read() calls a second with reads of 20KB (DVB HD content).

The combined result if this patch reduces CPU usage by ~15% on a 700MHz Cortex-A9 when recording DVB HD content.

Attachments (1)

mythtv_DeviceReadBuffer_reduce_thread_wakeup.patch (3.8 KB) - added by Rune Petersen <rune@…> 6 years ago.

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by Rune Petersen <rune@…>

comment:1 Changed 6 years ago by danielk

Owner: set to danielk
Status: newaccepted

comment:2 Changed 5 years ago by Daniel Thor Kristjansson <dkristjansson@…>

In 6912383b2c0c59df12aa9c8af521200fbe4e585d/mythtv:

Reduce # number DeviceReadBuffer::Read() calls.

Refs #11252.

This reduces the number of reads per second we make in the stream handlers
utilizing the DRB. It does this by waiting 20 ms for at least dev_read_size
bytes rather than 500 ms for min_read bytes. The device reads are larger
than the minimum read so this actually results in waiting a bit longer
under normal conditions while returning more quickly when we're not getting
data from the device. A win-win.

comment:3 Changed 5 years ago by danielk

Rune, I've made a modification similar to what you had for the Read call which reduces the number of times that call is made. However the change in the size of min_read goes against what we want to with those read, we want to get that data out of the very small OS buffers as quickly as possible hence the " Limit read size for faster return from read" comment and the code following it. Also the min_read you set was larger than the dev_read_size when not using poll which doesn't make much sense to me.

The streamhandler changes seemed harmless, but I didn't really see the point of them. Can you explain what you were doing there?

PS How much overall CPU usage are you seeing with the Cortex-A9? Improvements are always welcome, but my backend CPU usage during tests was too low to be measurable here on my old Intel E8400.

comment:4 Changed 5 years ago by danielk

Status: acceptedinfoneeded

comment:5 Changed 5 years ago by Rune Petersen <rune@…>

Your change will have no affect on will have no affect on wakeups.

I'll try to explain why: The current code will wait for at least 188*4 bytes of data and time out after (now) 20ms. if not enough data is received. This this gives you very low latency, at the cost of 700+ wakeups a second at bitrates over 6Mbps.

My change trades a little latency (worst case 20ms.) for a reduction in wakeups. try to read it as: wait for 20ms. unless the readThreshold is exceeded readThreshold should not be seen as a minimum read, but more as a flush threshold in case of starvation. The actual size of the latency is negotiable, even 10ms. will be noticeable.

The change to streamhandler is because of the reduced timeout in DeviceReadBuffer::Read() can result in Read() returning 0 bytes read in very low bitrate situations will will result in a 100ms. delay/latency spike. The old code waited 500ms. which makes this scenario more or less impossible.

CPU usage on my Cortex-A9 using unmodified MythTV is 36-40% when recording DVB HD content. (with all my patches (not just this one) CPU usage is ~10% )

Just for fun: my Cortex-A9 runs at 700MHz and has 128KB L2 Cache, your E8400 runs at 3GHz, and has 6MB L2 Cache.

comment:6 Changed 5 years ago by jpoet

Owner: changed from danielk to jpoet

comment:7 Changed 5 years ago by danielk

Status: infoneededassigned

Rune, I tested the changes I made and they do significantly reduce wakeups in the recorder thread. I think we may need to apply your stream handler changes to control latency, plus somehow rate control the reads from the devices to control wakeups in the DRB thread.

It is difficult for me to measure any CPU usage with my hardware. But I talked with John Poet who is experiencing high CPU usage as well, and he's taking this ticket.

comment:8 Changed 5 years ago by John Poet <jpoet@…>

Resolution: fixed
Status: assignedclosed

In 4f0bfd9d1eeff9397fe9386b905567b0e499dcb9/mythtv:

DeviceReadBuffer?: Try to optimize some of the busy-waiting.

Wait for readThreshold or 20 milliseconds before returning to the stream

Move the throttling "sleep" from the stream handler into the DRB run loop.

Original patch by Rune Petersen.
Closes #11252

comment:9 Changed 5 years ago by Raymond Wagner

Milestone: unknown0.27
Note: See TracTickets for help on using tickets.