Re: [vdr] [PATCH] dynamically sized ringbuffers v1

2007-05-09 Thread Artur Skawina
 void cRecorder::Receive(uchar *Data, int Length)
 {
   if (Running()) {
  int p = ringBuffer-Put(Data, Length);
  if (p != Length  Running())
 ringBuffer-ReportOverflow(Length - p);
  }
 }
 
 it simply drops any data that does not fit into the buffer, which would be 
 fine
 for live viewing, but isn't ideal when recording.
 
 Can we try harder not to loose data here? IOW can this function sleep (and 
 retry)?

It took a while to trigger the condition again, but now it happened and trying 
a bit
harder payed off. Running vdr /w following patch resulted in this log (and no 
overflow):

20:31:35 vdr: [16328] buffer usage: 70% (tid=16327)
20:31:35 vdr: [16328] buffer usage: 80% (tid=16327)
20:31:35 vdr: [16328] buffer usage: 90% (tid=16327)
20:31:35 vdr: [16328] buffer usage: 100% (tid=16327)
20:31:35 vdr: [16327] Enlarging ring buffer Result: 262144 bytes (trigger 3)
20:31:35 vdr: [16329] Enlarging ring buffer TS: 262144 bytes (trigger 2)
20:31:35 vdr: [16328] Enlarging ring buffer Recorder: 262144 bytes (trigger 3)
20:31:35 vdr: [16328] buffer usage: 0% (tid=16327)
20:31:35 vdr: [16328] saved extra 153 bytes in recorder ring buffer after 80 ms 
delay

artur

diff --git a/recorder.c b/recorder.c
index 8bb1621..3c0e002 100644
--- a/recorder.c
+++ b/recorder.c
@@ -157,8 +157,20 @@ void cRecorder::Receive(uchar *Data, int Length)
 {
   if (Running()) {
  int p = ringBuffer-Put(Data, Length);
- if (p != Length  Running())
+ if (p != Length  Running()) {
+for (int ms=20; ms1000; ms+=ms) {
+   cCondWait::SleepMs(ms);
+   if (!Running())
+  return;
+   int r = ringBuffer-Put(Data+p, Length-p);
+   p += r;
+   if (r)
+  dsyslog(saved extra %d bytes in recorder ring buffer after %d 
ms delay, r, ms);
+   if (p == Length || !Running())
+  return;
+   }
 ringBuffer-ReportOverflow(Length - p);
+}
  }
 }


___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


Re: [vdr] [PATCH] dynamically sized ringbuffers v1

2007-05-08 Thread Artur Skawina
 Patch below changes the interpretation of the 'Size' parm given when creating 
 a buffer from a
 fixed length to a max limit. The buffers start out much smaller (currently 
 128k, instead of many
 megabytes) and grow automatically when they become almost full. This way all 
 callers benefit,
 and they don't have to be modified at all.

 I've tested this on two vdrs for a few hours and it seems to work, but it 
 needs a lot
 more testing.

I did a few test recordings (total duration likely more than 24h) and the new 
code
seems mostly fine; just some tweaking of the sizes and grow heuristics left.

However one problem did appear after many hours of simultaneous recordings:

08:17:38 vdr: [24052] buffer usage: 70% (tid=24051)
08:17:39 vdr: [24052] buffer usage: 80% (tid=24051)
08:17:39 vdr: [24052] buffer usage: 90% (tid=24051)
08:17:39 vdr: [24052] buffer usage: 100% (tid=24051)
08:17:39 vdr: [24052] ERROR: 1 ring buffer overflow (61 bytes dropped)
08:17:45 vdr: [24052] ERROR: 9053 ring buffer overflows (1701964 bytes dropped)
08:17:47 vdr: [24052] Enlarging ring buffer Recorder: 442368 bytes
08:17:47 vdr: [24052] buffer usage: 0% (tid=24051)

Apparently the 'Recorder' consumer stalled for several seconds at a point when
growing the ringbuffer wasn't possible - to keep Get/Put lockless resizing is
only possible when the ringbuffer data is continuous; and if the consumer stops
making progress at the wrong moment, the buffer will not be enlarged and 
overflow.
In this case the buffer was ~300k and ~2m would have been needed.
I can think of a few ways to make the ringbuffer growing more aggressive, but
they all make the buffer handling much more complex, so i'd like to avoid that.

Looking at the caller:

void cRecorder::Receive(uchar *Data, int Length)
{
  if (Running()) {
 int p = ringBuffer-Put(Data, Length);
 if (p != Length  Running())
ringBuffer-ReportOverflow(Length - p);
 }
}

it simply drops any data that does not fit into the buffer, which would be fine
for live viewing, but isn't ideal when recording.

Can we try harder not to loose data here? IOW can this function sleep (and 
retry)?

(not that i think doing this would be enough; it could help for short stalls, 
but
for the longer ones a much larger buffer size is necessary, i'll have to find a
clean way to handle that anyway. It's kind of hard to test though as the 
overflow
only happened once during ~24h of recording...)

artur

___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


[vdr] [PATCH] dynamically sized ringbuffers v1

2007-05-07 Thread Artur Skawina
vdr uses quite a lot of memory, RSS here used to be ~70M, so i decided to see 
if it could be
reduced a bit. First suspects became the ringbuffers, cause i was seeing a lot 
of log lines
such as buffer stats: 96068 (1%) used. Turns out many buffers are way over 
sized, only a few
percent of the memory are typically used, yet because of how a ringbuffer works 
_all_ of it is
constantly accessed. I first tried to make the ringbuffer users smarter about 
picking the size
and eg after changing the transfer buffer to start out small and use bigger 
sizes only when the
current one isn't large enough it seemed a more reasonable size for that vdr 
instance was 384k
instead of 2M. Similarly the TS buffer could be just a few hundred K instead of 
2M.

But that approach was not ideal as there isn't anything one could do after 
realizing the buffer
is too small, it could be grown next time, after channel switch etc, but that's 
it. Also some
of the huge buffers came from plugins, and I wasn't all that excited about 
fixing all of them
(one example w/ the one-digit % fill was streamdev-server).
It is hard to estimate what the proper size would be under load, with many 
simultaneous recordings.
So I did attack this issue differently -- by making the ringbuffers themselves 
smarter.

Patch below changes the interpretation of the 'Size' parm given when creating a 
buffer from a
fixed length to a max limit. The buffers start out much smaller (currently 
128k, instead of many
megabytes) and grow automatically when they become almost full. This way all 
callers benefit,
and they don't have to be modified at all.

The buffers are actually always allocated using the max size, but only the 
necessary
buffer part is used, the rest of the memory is never accessed; this is done 
this way
so that no extra locking is necessary.

I've tested this on two vdrs for a few hours and it seems to work, but it needs 
a lot
more testing.
There is some extra logging in this patch so that you can see what's going on; 
every
time a ring buffer gets deleted it logs a line which tells you exactly how much 
memory
was requested and how much was really needed.

vdr: [19729] Deleting ring buffer TS size: 196608 / 2097152 (used 9.375% of 
requested size)
vdr: [19718] Deleting ring buffer Result size: 262144 / 262144 (used 100% of 
requested size)
vdr: [19718] Deleting ring buffer Recorder size: 442368 / 5242880 (used 
8.4375% of requested size)
vdr: [19718] Deleting ring buffer Result size: 262144 / 262144 (used 100% of 
requested size)
vdr: [19718] Deleting ring buffer Recorder size: 131072 / 5242880 (used 2.5% 
of requested size)
vdr: [19733] Deleting ring buffer TS size: 131072 / 2097152 (used 6.25% of 
requested size)
vdr: [19718] Deleting ring buffer Result size: 262144 / 262144 (used 100% of 
requested size)
vdr: [19718] Deleting ring buffer Recorder size: 442368 / 5242880 (used 
8.4375% of requested size)

So each of the several 'Recorder' buffers was an order of magnitude smaller 
than w/o
this patch and several megabytes less ram was used.

artur

diff --git a/ringbuffer.c b/ringbuffer.c
index 0633bd3..8dd1efe 100644
--- a/ringbuffer.c
+++ b/ringbuffer.c
@@ -151,13 +151,29 @@ void cRingBufferLinear::PrintDebugRBL(void)
   }
 #endif

+// cRingBufferLinear are dynamically sized, or at least we can pretend they 
are ;)
+// We treat 'Size' as the maximum size, but start with a small buffer, which 
can
+// grow later as it fills up. Memory is always allocated for the full buffer, 
but
+// the unused RAM portion remains untouched until (if at all) it is actually 
needed.
+// Note that we can not start with a larger than requested buffer because 
there are
+// ring buffer users that cause crashes then (eg softdevice mpegdecoder 
absolutely
+// needs 32/64k).
+
+// Startup size. 64k still causes overflows before buffer starts to grow, 128k 
doesn't.
+// The buffers grow to 200..500k anyway, so maybe increasing this a bit more 
would
+// make sense, but let's first see if it's really needed.
+#define DEFRBSIZE KILOBYTE(128)
+
 cRingBufferLinear::cRingBufferLinear(int Size, int Margin, bool Statistics, 
const char *Description)
-:cRingBuffer(Size, Statistics)
+:cRingBuffer(min(Size,DEFRBSIZE), Statistics)
 {
   description = Description ? strdup(Description) : NULL;
   tail = head = margin = Margin;
   gotten = 0;
   buffer = NULL;
+  maxsize = Size;
+  growbuf = 0;
+  dsyslog(New ring buffer \%s\ size: %d margin: %d, description, Size, 
Margin );
   if (Size  1) { // 'Size - 1' must not be 0!
  if (Margin = Size / 2) {
 buffer = MALLOC(uchar, Size);
@@ -183,6 +199,8 @@ cRingBufferLinear::~cRingBufferLinear()
 #ifdef DEBUGRINGBUFFERS
   DelDebugRBL(this);
 #endif
+  dsyslog(Deleting ring buffer \%s\ size: %d / %d (used %g%% of requested 
size), description,
+   size, maxsize, 
size/(double)maxsize*100.0 );
   free(buffer);
   free(description);
 }
@@ -207,6 +225,13 @@ void