Klaus Schmidinger wrote:
> Artur Skawina wrote:
>> this time with a new approach to read caching. Should make watching
>> and editing recordings on a non-idle (and/or slow) machine more
>> comfortable.
>>
>> The difference to previous versions (and stock fadvise-enabled vdr) is
>> that previously, read data was almost immediately forgotten after it
>> was used; now a certain amount of recently accessed data (at most
>> ~16M) is kept around. This means that short seeks (jumps) when
>> replaying do not cause disk accesses. Things like switching play mode,
>> FF, setting and moving editing marks shouldn't usually block waiting
>> for disk IO. The changes are most noticeable when eg. several
>> recordings are happening in the background.
>>
>> I did very little testing, treat this as beta quality at best. Seems
>> to work ok, but i won't probably have time to test it further for a
>> few days; maybe somebody wants to play w/ this, or even better take a
>> look at the Read() path...
after running w/ it for a week, i'd now say it works; haven't seen any problems
that could be related to the Read() code.
> Attached is the patch as I would add it for the next version.
Why not the inlined Seeks()? The IO is very inefficient anyway (tiny read/write
sizes), having an extra syscall (seek) for every read/write call doesn't help...
(yes, it may not be pretty, but it lets the compiler optimize the calls away,
w/o touching any callers. Unless you fixed the callers, then ignore this)
> Please take a look at the lines marked CLARIFY and let me know
> if you have any comment on these.
i'll have to rediff this and compare w/ my current version; will post merged
version later. Comments below.
> +++ cutter.c 2006/02/04 13:40:20
> @@ -66,6 +66,8 @@
> toFile = toFileName->Open();
> if (!fromFile || !toFile)
> return;
> + fromFile->SetReadAhead(MEGABYTE(20));
> + toFile->SetReadAhead(MEGABYTE(20));
> int Index = Mark->position;
> Mark = fromMarks.Next(Mark);
> int FileSize = 0;
> @@ -122,6 +125,7 @@
> error = "toFile 1";
> break;
> }
> + toFile->SetReadAhead(MEGABYTE(20));
> FileSize = 0;
> }
> LastIFrame = 0;
> @@ -162,6 +166,7 @@
> error = "toFile 2";
> break;
> }
> + toFile->SetReadAhead(MEGABYTE(20));
> FileSize = 0;
> }
> }
i've dropped the toFile->SetReadAhead() hack -- it doesn't change that much when
the datasyncs aren't there anyway.
> +++ tools.c 2006/02/04 13:55:24
> + // Read ahead:
> + // no jump? (allow small forward jump still inside readahead window).
> + if (jumped >= 0 && jumped <= (off_t)readahead) {
> + // Trigger the readahead IO, but only if we've used at least
> + // 1/2 of the previously requested area. This avoids calling
> + // fadvise() after every read() call.
> + if (ahead - curpos < (off_t)(readahead - readahead / 2)) {//XXX CLARIFY: isn't this just 'readahead / 2'???
historical reasons -- it used to be 1/4, not 1/2. I went back to 1/2 after
switching to the new read caching strategy; previously i was seeing stalls when
replaying and eg cutting at the same time. With the new code hopefully 1/2 is
enough and reduces the number of fadvise calls.
> + // The above fadvise() works when writing slowly (recording), but could
> + // leave cached data around when writing at a high rate (cutting).
> + // Also, it seems in some setups, the above does not trigger any I/O and
> + // the fdatasync() call below has to do all the work (reiserfs with some
> + // kind of write gathering enabled).
> + // We add 'readahead' to the threshold in an attempt to increase cutting
> + // speed; it's a tradeoff -- speed vs RAM-used.
> + if (totwritten > MEGABYTE(32) + readahead) {
> + //fdatasync(fd);//XXX CLARIFY: so what, is this necessary or not???
it is not, at least not here, however i left it commented out in case there are
setups where it actually helps -- it's easier to uncomment this one line for
testing than to find the right place to insert the call. It's not on by default
because fsyncs are relatively expensive; i'd rather avoid them unless absolutely
required. (i've updated the comments in this area since, they will be in the
merged version)
artur
[for faster responses please cc me; esp. if it's a reply to an older thread -- i
almost missed this]
