Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 17 months ago

Closed 15 months ago

Last modified 15 months 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

Description

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@…> 17 months ago.

Download all attachments as: .zip

Change History (10)

Changed 17 months ago by Rune Petersen <rune@…>

comment:1 Changed 17 months ago by danielk

  • Owner set to danielk
  • Status changed from new to accepted

comment:2 Changed 16 months 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 16 months 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 16 months ago by danielk

  • Status changed from accepted to infoneeded

comment:5 Changed 16 months 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 15 months ago by jpoet

  • Owner changed from danielk to jpoet

comment:7 Changed 15 months ago by danielk

  • Status changed from infoneeded to assigned

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 15 months ago by John Poet <jpoet@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 4f0bfd9d1eeff9397fe9386b905567b0e499dcb9/mythtv:

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

Wait for readThreshold or 20 milliseconds before returning to the stream
handler.

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

Original patch by Rune Petersen.
Closes #11252

comment:9 Changed 15 months ago by wagnerrp

  • Milestone changed from unknown to 0.27

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.