4th, hopefully final version.
Changes since v3:
o small bugfixes, cleanups and tuning (readahead size etc). o large reduction of numer of syscalls when doing IO. eg. VDR used to call lseek(2) twice (!) for every single read(2) call, mostly to confirm the current offset. We cache the file position now, which lets us avoid the lseeks completely when sequentially reading/writing. [I'm assuming there's never more than one user of a cUnbufferedFile object] o merging fadvice calls when sequentially reading. This means we free the used data in chunks of hundreds of kilobytes, instead of calling fadvise for every single read(2) call. o the manual readahead is done in much larger chunks too; this lets us avoid yet another fadvise() for every read(2) call. o the periodic fdatasync() calls are back. I realized they are mostly harmless when the streaming writeout works (since there is very little dirty data around). o a potential cache "leak" fixed. (i hope this was the last one) o the OSD write strategy selection is still around, but only the values '0' (UNBUFFERED) and '1' (BUFFERED) are supported (2 is equivalent to 0). (leaving this around until the unbuffered case is sufficiently tested)
Lightly tested on mostly artificial data, but seems to work ok; i didn't notice any cache leaks, even when cutting.
artur
--- vdr-1.3.36.org/config.c 2005-09-09 17:08:59.000000000 +0200 +++ vdr-1.3.36/config.c 2005-11-21 22:17:55.000000000 +0100 @@ -453,6 +453,7 @@ bool cSetup::Parse(const char *Name, con else if (!strcasecmp(Name, "UseSmallFont")) UseSmallFont = atoi(Value); else if (!strcasecmp(Name, "MaxVideoFileSize")) MaxVideoFileSize = atoi(Value); else if (!strcasecmp(Name, "SplitEditedFiles")) SplitEditedFiles = atoi(Value); + else if (!strcasecmp(Name, "WriteStrategy")) WriteStrategy = atoi(Value); else if (!strcasecmp(Name, "MinEventTimeout")) MinEventTimeout = atoi(Value); else if (!strcasecmp(Name, "MinUserInactivity")) MinUserInactivity = atoi(Value); else if (!strcasecmp(Name, "MultiSpeedMode")) MultiSpeedMode = atoi(Value); @@ -518,6 +519,7 @@ bool cSetup::Save(void) Store("UseSmallFont", UseSmallFont); Store("MaxVideoFileSize", MaxVideoFileSize); Store("SplitEditedFiles", SplitEditedFiles); + Store("WriteStrategy", WriteStrategy); Store("MinEventTimeout", MinEventTimeout); Store("MinUserInactivity", MinUserInactivity); Store("MultiSpeedMode", MultiSpeedMode); --- vdr-1.3.36.org/config.h 2005-11-04 16:55:05.000000000 +0100 +++ vdr-1.3.36/config.h 2005-11-21 21:57:52.000000000 +0100 @@ -249,6 +249,7 @@ public: int UseSmallFont; int MaxVideoFileSize; int SplitEditedFiles; + int WriteStrategy; int MinEventTimeout, MinUserInactivity; int MultiSpeedMode; int ShowReplayMode; --- vdr-1.3.36.org/cutter.c 2005-10-31 13:26:44.000000000 +0100 +++ vdr-1.3.36/cutter.c 2005-11-22 19:29:03.000000000 +0100 @@ -66,6 +66,7 @@ void cCuttingThread::Action(void) toFile = toFileName->Open(); if (!fromFile || !toFile) return; + fromFile->setreadahead(MEGABYTE(5)); int Index = Mark->position; Mark = fromMarks.Next(Mark); int FileSize = 0; @@ -90,6 +91,7 @@ void cCuttingThread::Action(void) if (fromIndex->Get(Index++, &FileNumber, &FileOffset, &PictureType, &Length)) { if (FileNumber != CurrentFileNumber) { fromFile = fromFileName->SetOffset(FileNumber, FileOffset); + fromFile->setreadahead(MEGABYTE(5)); CurrentFileNumber = FileNumber; } if (fromFile) { --- vdr-1.3.36.org/menu.c 2005-11-05 18:29:22.000000000 +0100 +++ vdr-1.3.36/menu.c 2005-11-21 22:14:38.000000000 +0100 @@ -2270,6 +2270,7 @@ cMenuSetupRecord::cMenuSetupRecord(void) Add(new cMenuEditIntItem( tr("Setup.Recording$Instant rec. time (min)"), &data.InstantRecordTime, 1, MAXINSTANTRECTIME)); Add(new cMenuEditIntItem( tr("Setup.Recording$Max. video file size (MB)"), &data.MaxVideoFileSize, MINVIDEOFILESIZE, MAXVIDEOFILESIZE)); Add(new cMenuEditBoolItem(tr("Setup.Recording$Split edited files"), &data.SplitEditedFiles)); + Add(new cMenuEditIntItem( tr("Setup.Recording$Write strategy"), &data.WriteStrategy, 0, 2)); }
// --- cMenuSetupReplay ------------------------------------------------------ --- vdr-1.3.36.org/tools.c 2005-11-04 17:33:18.000000000 +0100 +++ vdr-1.3.36/tools.c 2005-11-22 19:30:25.000000000 +0100 @@ -7,6 +7,7 @@ * $Id: tools.c 1.103 2005/11/04 16:33:18 kls Exp $ */
+#include "config.h" #include "tools.h" #include <ctype.h> #include <dirent.h> @@ -851,8 +852,7 @@ bool cSafeFile::Close(void)
// --- cUnbufferedFile -------------------------------------------------------
-#define READ_AHEAD MEGABYTE(2) -#define WRITE_BUFFER MEGABYTE(10) +#define WRITE_BUFFER KILOBYTE(800)
cUnbufferedFile::cUnbufferedFile(void) { @@ -868,23 +868,28 @@ int cUnbufferedFile::Open(const char *Fi { Close(); fd = open(FileName, Flags, Mode); - begin = end = ahead = -1; + curpos = 0; + begin = lastpos = ahead = 0; + readahead = 128*1024; + pendingreadahead = 0; written = 0; + totwritten = 0; + if (fd >= 0) { + // we really mean POSIX_FADV_SEQUENTIAL, but we do our own readahead + // so turn off the kernel one. + if (Setup.WriteStrategy!=1) + posix_fadvise(fd, 0, 0, POSIX_FADV_RANDOM); + } return fd; }
int cUnbufferedFile::Close(void) { if (fd >= 0) { - if (ahead > end) - end = ahead; - if (begin >= 0 && end > begin) { - //dsyslog("close buffer: %d (flush: %d bytes, %ld-%ld)", fd, written, begin, end); - if (written) - fdatasync(fd); - posix_fadvise(fd, begin, end - begin, POSIX_FADV_DONTNEED); - } - begin = end = ahead = -1; + if (totwritten) + fdatasync(fd); + posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED); + begin = lastpos = ahead = 0; written = 0; } int OldFd = fd; @@ -894,40 +899,105 @@ int cUnbufferedFile::Close(void)
off_t cUnbufferedFile::Seek(off_t Offset, int Whence) { - if (fd >= 0) - return lseek(fd, Offset, Whence); - return -1; + //dsyslog("Seek: fd: %d offs: %ld whence: %d diff: %ld", fd, (long)Offset, Whence, Offset-curpos); + if (Whence == SEEK_SET && Offset == curpos) + return curpos; + + curpos = lseek(fd, Offset, Whence); + return curpos; }
+// when replaying and going eg FF->PLAY the position jumps back 2..8M +// hence we might not want to drop that data at once. +// Ignoring for now to avoid making this even more complex, but we could +// at least try to handle the common cases +// (PLAY->FF->PLAY, small jumps, moving editing marks etc) + ssize_t cUnbufferedFile::Read(void *Data, size_t Size) { if (fd >= 0) { - off_t pos = lseek(fd, 0, SEEK_CUR); - // jump forward - adjust end position - if (pos > end) - end = pos; - // after adjusting end - don't clear more than previously requested - if (end > ahead) - end = ahead; - // jump backward - drop read ahead of previous run - if (pos < begin) - end = ahead; - if (begin >= 0 && end > begin) - posix_fadvise(fd, begin - KILOBYTE(200), end - begin + KILOBYTE(200), POSIX_FADV_DONTNEED);//XXX macros/parameters??? - begin = pos; + off_t jumped = curpos-lastpos; // nonzero means we're not at the last offset - some kind of jump happened. + if (jumped) { + pendingreadahead += ahead-lastpos+MEGABYTE(4); + // jumped forward? - treat as if we did read all the way to current pos. + if (jumped >= 0) { + lastpos = curpos; + // but clamp at ahead so we don't clear more than previously requested. + // (would be mostly harmless anyway, unless we got more than one reader of this file) + // add a little extra readahead, JIC the kernel prefethed more than we requested. + if (lastpos > (ahead+MEGABYTE(4))) + lastpos = ahead+MEGABYTE(4); + } + // jumped backward? - drop both last read _and_ read-ahead + else + if (curpos < begin) + lastpos = ahead+MEGABYTE(4); + // jumped backward, but still inside prev read window? - pretend we read less. + else /* if (curpos >= begin) */ + lastpos = curpos; + } + ssize_t bytesRead = safe_read(fd, Data, Size); + + // now drop all data accesed during _previous_ Read(). + // but try to merge the fadvise calls a bit in order to reduce overhead. + if (Setup.WriteStrategy!=1 && begin >= 0 && lastpos > begin) + if (jumped || (size_t)(lastpos-begin) > readahead) { + //dsyslog("taildrop: %ld..%ld size %ld", begin, lastpos, lastpos-begin); + posix_fadvise(fd, begin-4095, lastpos-begin+4095*2, POSIX_FADV_DONTNEED); + begin = curpos; + } + if (bytesRead > 0) { - pos += bytesRead; - end = pos; + curpos += bytesRead; // this seems to trigger a non blocking read - this // may or may not have been finished when we will be called next time. // If it is not finished we can't release the not yet filled buffers. // So this is commented out till we find a better solution. - //posix_fadvise(fd, pos, READ_AHEAD, POSIX_FADV_WILLNEED); - ahead = pos + READ_AHEAD; + + // Hmm, it's obviously harmless if we're actually going to read the data + // -- the whole point of read-ahead is to start the IO early... + // The comment above applies only when we jump somewhere else _before_ the + // IO started here finishes. How common would that be? Could be handled eg + // by posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED) called some time after + // we detect a jump. Ignoring this for now. /AS + + // Ugh, it seems to cause some "leaks" at every jump... Either the + // brute force approach mentioned above should work (it's not like this is + // much different than O_DIRECT) or keeping notes about the ahead reads and + // flushing them after some time. the latter seems overkill though, trying + // the former... + + //dsyslog("jump: %06ld ra: %06ld size: %ld", jumped, (long)readahead, (long)Size); + + // no jump? also permit small jump still inside readahead window (FF). + if (Setup.WriteStrategy!=1 && jumped>=0 && jumped<=(off_t)readahead) { + if ( readahead < Size*32 ) { // automagically tune readahead size. + readahead = Size*32; + //dsyslog("Readahead for fd: %d increased to %ld", fd, (long)readahead); + } + if (ahead<(off_t)(curpos+readahead/2)) { + posix_fadvise(fd, curpos, readahead, POSIX_FADV_WILLNEED); + ahead = curpos + readahead; + } + } + else { + // jumped - we really don't want any readahead now. otherwise + // eg fast-rewind gets in trouble. + ahead = curpos; + + // flush it all; mostly to get rid of nonflushed readahead coming + // from _previous_ jumps. ratelimited. + // the accounting is _very_ unaccurate, i've seen ~50M get flushed + // when the limit was set to 4M. As long as this triggers after + // _some_ jumps we should be ok though. + if (Setup.WriteStrategy!=1 && pendingreadahead>MEGABYTE(20)) { + pendingreadahead = 0; + posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED); + } + } } - else - end = pos; + lastpos = curpos; return bytesRead; } return -1; @@ -936,25 +1006,35 @@ ssize_t cUnbufferedFile::Read(void *Data ssize_t cUnbufferedFile::Write(const void *Data, size_t Size) { if (fd >=0) { - off_t pos = lseek(fd, 0, SEEK_CUR); ssize_t bytesWritten = safe_write(fd, Data, Size); if (bytesWritten >= 0) { + curpos += bytesWritten; written += bytesWritten; if (begin >= 0) { - if (pos < begin) - begin = pos; + if (curpos < begin) + begin = curpos; } else - begin = pos; - if (pos + bytesWritten > end) - end = pos + bytesWritten; + begin = curpos; + if (curpos + bytesWritten > lastpos) + lastpos = curpos + bytesWritten; if (written > WRITE_BUFFER) { - //dsyslog("flush buffer: %d (%d bytes, %ld-%ld)", fd, written, begin, end); - fdatasync(fd); - if (begin >= 0 && end > begin) - posix_fadvise(fd, begin, end - begin, POSIX_FADV_DONTNEED); - begin = end = -1; + //dsyslog("flush buffer: %d (%d bytes, %ld-%ld)", fd, written, begin, lastpos); + totwritten += written; + if (Setup.WriteStrategy!=1 && begin >= 0 && lastpos > begin) { + off_t headdrop = max(begin,WRITE_BUFFER*2L); + posix_fadvise(fd, begin-headdrop, lastpos - begin + headdrop, POSIX_FADV_DONTNEED); + } + begin = lastpos = 0; written = 0; + // the above fadvise() works when recording, but seems to leave cached + // data around when writing at a high rate (eg cutting). Hence... + if (Setup.WriteStrategy!=1 && totwritten>MEGABYTE(20)) { + //if (Setup.WriteStrategy==2) + fdatasync(fd); + posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED); + totwritten = 0; + } } } return bytesWritten; --- vdr-1.3.36.org/tools.h 2005-11-05 11:54:39.000000000 +0100 +++ vdr-1.3.36/tools.h 2005-11-22 16:16:16.000000000 +0100 @@ -205,10 +205,14 @@ public: class cUnbufferedFile { private: int fd; + off_t curpos; off_t begin; - off_t end; + off_t lastpos; off_t ahead; + size_t pendingreadahead; + size_t readahead; ssize_t written; + ssize_t totwritten; public: cUnbufferedFile(void); ~cUnbufferedFile(); @@ -218,6 +222,7 @@ public: ssize_t Read(void *Data, size_t Size); ssize_t Write(const void *Data, size_t Size); static cUnbufferedFile *Create(const char *FileName, int Flags, mode_t Mode = DEFFILEMODE); + void setreadahead(size_t ra) { readahead = ra; }; };
class cLockFile {