avoid extra reallocations and memcpy for seekable files in OnRead() (slightly modified patch 1935975)

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@53557 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

src/common/textfile.cpp

@@ -92,15 +92,17 @@ bool wxTextFile::OnRead(const wxMBConv& conv)
     wxASSERT_MSG( m_file.IsOpened(), _T("can't read closed file") );
 
     // read the entire file in memory: this is not the most efficient thing to
-    // do but there is no good way to avoid it in Unicode build because if we
+    // do it but there is no good way to avoid it in Unicode build because if
-    // read the file block by block we can't convert each block to Unicode
+    // we read the file block by block we can't convert each block to Unicode
     // separately (the last multibyte char in the block might be only partially
     // read and so the conversion would fail) and, as the file contents is kept
     // in memory by wxTextFile anyhow, it shouldn't be a big problem to read
     // the file entirely
-    size_t bufSize = 0,
+    size_t bufSize = 0;
-           bufPos = 0;
+
-    char block[1024];
+    // number of bytes to (try to) read from disk at once
+    static const size_t BLOCK_SIZE = 4096;
+
     wxCharBuffer buf;
 
     // first determine if the file is seekable or not and so whether we can
@@ -118,57 +120,70 @@ bool wxTextFile::OnRead(const wxMBConv& conv)
     {
         // we know the required length, so set the buffer size in advance
         bufSize = fileLength;
-        if ( !buf.extend(bufSize - 1 /* it adds 1 internally */) )
+        if ( !buf.extend(bufSize) )
             return false;
 
         // if the file is seekable, also check that we're at its beginning
         wxASSERT_MSG( m_file.Tell() == 0, _T("should be at start of file") );
-    }
 
-    for ( ;; )
+        char *dst = buf.data();
-    {
+        for ( ;; )
-        ssize_t nRead = m_file.Read(block, WXSIZEOF(block));
-
-        if ( nRead == wxInvalidOffset )
         {
-            // read error (error message already given in wxFile::Read)
+            ssize_t nRead = m_file.Read(dst, BLOCK_SIZE);
-            return false;
-        }
 
-        if ( nRead == 0 )
+            if ( nRead == wxInvalidOffset )
-        {
+            {
-            // if no bytes have been read, presumably this is a valid-but-empty file
+                // read error (error message already given in wxFile::Read)
-            if ( bufPos == 0 )
-                return true;
-
-            // otherwise we've finished reading the file
-            break;
-        }
-
-        if ( seekable )
-        {
-            // this shouldn't happen but don't overwrite the buffer if it does
-            wxCHECK_MSG( bufPos + nRead <= bufSize, false,
-                         _T("read more than file length?") );
-        }
-        else // !seekable
-        {
-            // for non-seekable files we have to allocate more memory on the go
-            if ( !buf.extend(bufPos + nRead - 1 /* it adds 1 internally */) )
                 return false;
+            }
+
+            if ( nRead == 0 )
+            {
+                // this file can't be empty because we checked for this above
+                break;
+            }
+
+            dst += nRead;
         }
 
-        // append to the buffer
+        wxASSERT_MSG( dst - buf.data() == (wxFileOffset)bufSize,
-        memcpy(buf.data() + bufPos, block, nRead);
+                      _T("logic error") );
-        bufPos += nRead;
     }
-
+    else // file is not seekable
-    if ( !seekable )
     {
-        bufSize = bufPos;
+        char block[BLOCK_SIZE];
+        for ( ;; )
+        {
+            ssize_t nRead = m_file.Read(block, WXSIZEOF(block));
+
+            if ( nRead == wxInvalidOffset )
+            {
+                // read error (error message already given in wxFile::Read)
+                return false;
+            }
+
+            if ( nRead == 0 )
+            {
+                // if no bytes have been read, presumably this is a
+                // valid-but-empty file
+                if ( bufSize == 0 )
+                    return true;
+
+                // otherwise we've finished reading the file
+                break;
+            }
+
+            // extend the buffer for new data
+            if ( !buf.extend(bufSize + nRead) )
+                return false;
+
+            // and append it to the buffer
+            memcpy(buf.data() + bufSize, block, nRead);
+            bufSize += nRead;
+        }
     }
 
-    const wxString str(buf, conv, bufPos);
+    const wxString str(buf, conv, bufSize);
 
     // there's no risk of this happening in ANSI build
 #if wxUSE_UNICODE
@@ -179,7 +194,8 @@ bool wxTextFile::OnRead(const wxMBConv& conv)
     }
 #endif // wxUSE_UNICODE
 
-    free(buf.release()); // we don't need this memory any more
+    // we don't need this memory any more
+    buf.reset();
 
 
     // now break the buffer in lines