From 96c05aae3402adc200bc6a49f85fdf39b58cdc9a Mon Sep 17 00:00:00 2001 From: Glenn Maynard Date: Tue, 28 Feb 2006 07:43:54 +0000 Subject: [PATCH] Move write buffering from RageFileObjDirect to RageFileObj; enable it for RageFileDriverTimeout, too. --- stepmania/src/RageFileBasic.cpp | 87 ++++++++++++++++++++++++- stepmania/src/RageFileBasic.h | 11 ++++ stepmania/src/RageFileDriverDirect.cpp | 48 +++----------- stepmania/src/RageFileDriverTimeout.cpp | 6 ++ 4 files changed, 113 insertions(+), 39 deletions(-) diff --git a/stepmania/src/RageFileBasic.cpp b/stepmania/src/RageFileBasic.cpp index 1aa02afd85..631dfcb5f2 100644 --- a/stepmania/src/RageFileBasic.cpp +++ b/stepmania/src/RageFileBasic.cpp @@ -5,10 +5,13 @@ RageFileObj::RageFileObj() { m_pReadBuffer = NULL; + m_pWriteBuffer = NULL; ResetReadBuf(); m_iReadBufAvail = 0; + m_iWriteBufferPos = 0; + m_iWriteBufferUsed = 0; m_bEOF = false; m_iFilePos = 0; m_bCRC32Enabled = false; @@ -32,6 +35,19 @@ RageFileObj::RageFileObj( const RageFileObj &cpy ): m_pReadBuffer = NULL; } + if( cpy.m_pWriteBuffer != NULL ) + { + m_pWriteBuffer = new char[cpy.m_iWriteBufferSize]; + m_iWriteBufferPos = cpy.m_iWriteBufferPos; + m_iWriteBufferUsed = cpy.m_iWriteBufferUsed; + m_iWriteBufferSize = cpy.m_iWriteBufferSize; + memcpy( m_pWriteBuffer, cpy.m_pWriteBuffer, m_iWriteBufferUsed ); + } + else + { + m_pWriteBuffer = NULL; + } + m_iReadBufAvail = cpy.m_iReadBufAvail; m_bEOF = cpy.m_bEOF; m_iFilePos = cpy.m_iFilePos; @@ -42,6 +58,7 @@ RageFileObj::RageFileObj( const RageFileObj &cpy ): RageFileObj::~RageFileObj() { delete [] m_pReadBuffer; + delete [] m_pWriteBuffer; } int RageFileObj::Seek( int iOffset ) @@ -56,6 +73,9 @@ int RageFileObj::Seek( int iOffset ) /* If we're calculating a CRC32, disable it. */ m_bCRC32Enabled = false; + /* Note that seeks do not flush the write buffer. Instead, we flush lazily, on the next + * actual Write (or Flush). Seek is not allowed to fail, and users should not need to + * flush before seeking to do proper error checking. */ ResetReadBuf(); int iPos = SeekInternal( iOffset ); @@ -178,8 +198,61 @@ int RageFileObj::Read( void *pBuffer, size_t iBytes, int iNmemb ) return iRet/iBytes; } +/* Empty the write buffer to disk. Return -1 on error, 0 on success. */ +int RageFileObj::EmptyWriteBuf() +{ + if( m_pWriteBuffer == NULL ) + return 0; + + /* The write buffer may not align with the actual file, if we've seeked. Only + * seek if needed. */ + bool bSeeked = (m_iWriteBufferPos+m_iWriteBufferUsed != m_iFilePos); + if( bSeeked ) + SeekInternal( m_iWriteBufferPos ); + + int iRet = WriteInternal( m_pWriteBuffer, m_iWriteBufferUsed ); + + if( bSeeked ) + SeekInternal( m_iFilePos ); + if( iRet == -1 ) + return iRet; + + m_iWriteBufferPos = m_iFilePos; + m_iWriteBufferUsed = 0; + return 0; +} + int RageFileObj::Write( const void *pBuffer, size_t iBytes ) { + if( m_pWriteBuffer != NULL ) + { + /* If the file position has moved away from the write buffer, or the + * incoming data won't fit in the buffer, flush. */ + if( m_iWriteBufferUsed ) + { + if( m_iWriteBufferPos+m_iWriteBufferUsed != m_iFilePos || m_iWriteBufferUsed + (int)iBytes > m_iWriteBufferSize ) + { + int iRet = EmptyWriteBuf(); + if( iRet == -1 ) + return iRet; + } + } + + if( m_iWriteBufferUsed + (int)iBytes <= m_iWriteBufferSize ) + { + memcpy( m_pWriteBuffer+m_iWriteBufferUsed, pBuffer, iBytes ); + m_iWriteBufferUsed += iBytes; + m_iFilePos += iBytes; + if( m_bCRC32Enabled ) + CRC32( m_iCRC32, pBuffer, iBytes ); + return iBytes; + } + + /* We're writing a lot of data, and it won't fit in the buffer. We already + * flushed above, so m_iWriteBufferUsed; fall through and write the block normally. */ + ASSERT_M( m_iWriteBufferUsed == 0, ssprintf("%i", m_iWriteBufferUsed) ); + } + int iRet = WriteInternal( pBuffer, iBytes ); if( iRet != -1 ) { @@ -201,16 +274,28 @@ int RageFileObj::Write( const void *pBuffer, size_t iBytes, int iNmemb ) int RageFileObj::Flush() { + int iRet = EmptyWriteBuf(); + if( iRet == -1 ) + return iRet; return FlushInternal(); } - void RageFileObj::EnableReadBuffering() { if( m_pReadBuffer == NULL ) m_pReadBuffer = new char[BSIZE]; } +void RageFileObj::EnableWriteBuffering( int iBytes ) +{ + if( m_pWriteBuffer == NULL ) + { + m_pWriteBuffer = new char[iBytes]; + m_iWriteBufferPos = m_iFilePos; + m_iWriteBufferSize = iBytes; + } +} + void RageFileObj::EnableCRC32( bool bOn ) { if( !bOn ) diff --git a/stepmania/src/RageFileBasic.h b/stepmania/src/RageFileBasic.h index 5841a74e4a..ed10f26b14 100644 --- a/stepmania/src/RageFileBasic.h +++ b/stepmania/src/RageFileBasic.h @@ -98,6 +98,7 @@ protected: virtual int FlushInternal() { return 0; } void EnableReadBuffering(); + void EnableWriteBuffering( int iBytes=1024*64 ); void SetError( const RString &sError ) { m_sError = sError; } RString m_sError; @@ -105,6 +106,7 @@ protected: private: int FillReadBuf(); void ResetReadBuf(); + int EmptyWriteBuf(); bool m_bEOF; int m_iFilePos; @@ -130,6 +132,15 @@ private: char *m_pReadBuf; int m_iReadBufAvail; + /* + * If write buffering is enabled, m_pWriteBuffer will be allocated, and m_iWriteBufferPos + * is the file position of the start of the buffer. + */ + char *m_pWriteBuffer; + int m_iWriteBufferPos; + int m_iWriteBufferSize; + int m_iWriteBufferUsed; + /* If EnableCRC32() is called, a CRC32 will be calculated as the file is read. * This is only meaningful if EnableCRC32() is called at the very start of the * file, and no seeking is performed. */ diff --git a/stepmania/src/RageFileDriverDirect.cpp b/stepmania/src/RageFileDriverDirect.cpp index e516c3f81f..a6951ab681 100644 --- a/stepmania/src/RageFileDriverDirect.cpp +++ b/stepmania/src/RageFileDriverDirect.cpp @@ -48,7 +48,6 @@ private: int m_iFD; int m_iMode; RString m_sPath; /* for Copy */ - RString m_sWriteBuf; /* * When not streaming to disk, we write to a temporary file, and rename to the @@ -218,7 +217,7 @@ RageFileObjDirect::RageFileObjDirect( const RString &sPath, int iFD, int iMode ) ASSERT( m_iFD != -1 ); if( m_iMode & RageFile::WRITE ) - m_sWriteBuf.reserve( BUFSIZE ); + this->EnableWriteBuffering( BUFSIZE ); } bool RageFileObjDirect::FinalFlush() @@ -364,21 +363,7 @@ int RageFileObjDirect::FlushInternal() return -1; } - if( !m_sWriteBuf.size() ) - return 0; - - int iRet = RetriedWrite( m_iFD, m_sWriteBuf.data(), m_sWriteBuf.size() ); - if( iRet == -1 ) - { - LOG->Warn("Error writing %s: %s", this->m_sPath.c_str(), strerror(errno) ); - SetError( strerror(errno) ); - m_bWriteFailed = true; - return -1; - } - - m_sWriteBuf.erase(); - m_sWriteBuf.reserve( BUFSIZE ); - return iRet; + return 0; } int RageFileObjDirect::WriteInternal( const void *pBuf, size_t iBytes ) @@ -389,29 +374,16 @@ int RageFileObjDirect::WriteInternal( const void *pBuf, size_t iBytes ) return -1; } - if( m_sWriteBuf.size()+iBytes > BUFSIZE ) + /* The buffer is cleared. If we still don't have space, it's bigger than + * the buffer size, so just write it directly. */ + int iRet = RetriedWrite( m_iFD, pBuf, iBytes ); + if( iRet == -1 ) { - if( FlushInternal() == -1 ) - return -1; - ASSERT( !m_sWriteBuf.size() ); - - /* The buffer is cleared. If we still don't have space, it's bigger than - * the buffer size, so just write it directly. */ - if( iBytes >= BUFSIZE ) - { - int iRet = RetriedWrite( m_iFD, pBuf, iBytes ); - if( iRet == -1 ) - { - LOG->Warn("Error writing %s: %s", this->m_sPath.c_str(), strerror(errno) ); - SetError( strerror(errno) ); - m_bWriteFailed = true; - return -1; - } - return iBytes; - } + LOG->Warn("Error writing %s: %s", this->m_sPath.c_str(), strerror(errno) ); + SetError( strerror(errno) ); + m_bWriteFailed = true; + return -1; } - - m_sWriteBuf.append( (const char *) pBuf, (const char *) pBuf+iBytes ); return iBytes; } diff --git a/stepmania/src/RageFileDriverTimeout.cpp b/stepmania/src/RageFileDriverTimeout.cpp index 6b39fbc2a7..d8ac789707 100644 --- a/stepmania/src/RageFileDriverTimeout.cpp +++ b/stepmania/src/RageFileDriverTimeout.cpp @@ -723,6 +723,10 @@ protected: int ReadInternal( void *pBuffer, size_t iBytes ) { + /* We have a lot of overhead per read and write operation, since we send + * commands to the worker thread. Buffer these operations. */ + EnableReadBuffering(); + RString sError; int iRet = m_pWorker->Read( m_pFile, pBuffer, iBytes, sError ); @@ -740,6 +744,8 @@ protected: int WriteInternal( const void *pBuffer, size_t iBytes ) { + EnableWriteBuffering(); + RString sError; int iRet = m_pWorker->Write( m_pFile, pBuffer, iBytes, sError );