On 1.8.x branch: Merge r1806299, r1806301, r1806308, r1806610:

  *) apr_file_write: Optimize large writes to buffered files on
     Windows.

git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.8.x@1902378 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 279 insertions, 53 deletions

diff --git a/CHANGES b/CHANGES
index 675365611..adc000fcd 100644
--- a/CHANGES
+++ b/CHANGES
@@ -23,6 +23,9 @@ Changes for APR 1.8.0
   *) Don't seek to the end when opening files with APR_FOPEN_APPEND on Windows.
      [Evgeny Kotkov <evgeny.kotkov visualsvn.com>]
 
+  *) apr_file_write: Optimize large writes to buffered files on Windows.
+     [Evgeny Kotkov <evgeny.kotkov visualsvn.com>]
+
 Changes for APR 1.7.1
 
   *) SECURITY: CVE-2021-35940 (cve.mitre.org)
diff --git a/file_io/win32/readwrite.c b/file_io/win32/readwrite.c
index efa5b04c1..b13b230e9 100644
--- a/file_io/win32/readwrite.c
+++ b/file_io/win32/readwrite.c
@@ -247,6 +247,91 @@ APR_DECLARE(apr_status_t) apr_file_read(apr_file_t *thefile, void *buf, apr_size
     return rv;
 }
 
+/* Helper function that adapts WriteFile() to apr_size_t instead
+ * of DWORD. */
+static apr_status_t write_helper(HANDLE filehand, const char *buf,
+                                 apr_size_t len, apr_size_t *pwritten)
+{
+    apr_size_t remaining = len;
+
+    *pwritten = 0;
+    do {
+        DWORD to_write;
+        DWORD written;
+
+        if (remaining > APR_DWORD_MAX) {
+            to_write = APR_DWORD_MAX;
+        }
+        else {
+            to_write = (DWORD)remaining;
+        }
+
+        if (!WriteFile(filehand, buf, to_write, &written, NULL)) {
+            *pwritten += written;
+            return apr_get_os_error();
+        }
+
+        *pwritten += written;
+        remaining -= written;
+        buf += written;
+    } while (remaining);
+
+    return APR_SUCCESS;
+}
+
+static apr_status_t write_buffered(apr_file_t *thefile, const char *buf,
+                                   apr_size_t len, apr_size_t *pwritten)
+{
+    apr_status_t rv;
+
+    if (thefile->direction == 0) {
+        /* Position file pointer for writing at the offset we are logically reading from */
+        apr_off_t offset = thefile->filePtr - thefile->dataRead + thefile->bufpos;
+        DWORD offlo = (DWORD)offset;
+        LONG offhi = (LONG)(offset >> 32);
+        if (offset != thefile->filePtr)
+            SetFilePointer(thefile->filehand, offlo, &offhi, FILE_BEGIN);
+        thefile->bufpos = thefile->dataRead = 0;
+        thefile->direction = 1;
+    }
+
+    *pwritten = 0;
+
+    while (len > 0) {
+        if (thefile->bufpos == thefile->bufsize) { /* write buffer is full */
+            rv = apr_file_flush(thefile);
+            if (rv) {
+                return rv;
+            }
+        }
+        /* If our buffer is empty, and we cannot fit the remaining chunk
+         * into it, write the chunk with a single syscall and return.
+         */
+        if (thefile->bufpos == 0 && len > thefile->bufsize) {
+            apr_size_t written;
+
+            rv = write_helper(thefile->filehand, buf, len, &written);
+            thefile->filePtr += written;
+            *pwritten += written;
+            return rv;
+        }
+        else {
+            apr_size_t blocksize = len;
+
+            if (blocksize > thefile->bufsize - thefile->bufpos) {
+                blocksize = thefile->bufsize - thefile->bufpos;
+            }
+            memcpy(thefile->buffer + thefile->bufpos, buf, blocksize);
+            thefile->bufpos += blocksize;
+            buf += blocksize;
+            len -= blocksize;
+            *pwritten += blocksize;
+        }
+    }
+
+    return APR_SUCCESS;
+}
+
 APR_DECLARE(apr_status_t) apr_file_write(apr_file_t *thefile, const void *buf, apr_size_t *nbytes)
 {
     apr_status_t rv;
@@ -267,38 +352,10 @@ APR_DECLARE(apr_status_t) apr_file_write(apr_file_t *thefile, const void *buf, a
     }
 
     if (thefile->buffered) {
-        char *pos = (char *)buf;
-        apr_size_t blocksize;
-        apr_size_t size = *nbytes;
-
         if (thefile->flags & APR_FOPEN_XTHREAD) {
             apr_thread_mutex_lock(thefile->mutex);
         }
-
-        if (thefile->direction == 0) {
-            /* Position file pointer for writing at the offset we are logically reading from */
-            apr_off_t offset = thefile->filePtr - thefile->dataRead + thefile->bufpos;
-            DWORD offlo = (DWORD)offset;
-            LONG  offhi = (LONG)(offset >> 32);
-            if (offset != thefile->filePtr)
-                SetFilePointer(thefile->filehand, offlo, &offhi, FILE_BEGIN);
-            thefile->bufpos = thefile->dataRead = 0;
-            thefile->direction = 1;
-        }
-
-        rv = 0;
-        while (rv == 0 && size > 0) {
-            if (thefile->bufpos == thefile->bufsize)   /* write buffer is full */
-                rv = apr_file_flush(thefile);
-
-            blocksize = size > thefile->bufsize - thefile->bufpos ? 
-                                     thefile->bufsize - thefile->bufpos : size;
-            memcpy(thefile->buffer + thefile->bufpos, pos, blocksize);
-            thefile->bufpos += blocksize;
-            pos += blocksize;
-            size -= blocksize;
-        }
-
+        rv = write_buffered(thefile, buf, *nbytes, nbytes);
         if (thefile->flags & APR_FOPEN_XTHREAD) {
             apr_thread_mutex_unlock(thefile->mutex);
         }
@@ -599,34 +656,14 @@ APR_DECLARE(apr_status_t) apr_file_gets(char *str, int len, apr_file_t *thefile)
 APR_DECLARE(apr_status_t) apr_file_flush(apr_file_t *thefile)
 {
     if (thefile->buffered) {
-        DWORD numbytes, written = 0;
         apr_status_t rc = 0;
-        char *buffer;
-        apr_size_t bytesleft;
 
         if (thefile->direction == 1 && thefile->bufpos) {
-            buffer = thefile->buffer;
-            bytesleft = thefile->bufpos;           
-
-            do {
-                if (bytesleft > APR_DWORD_MAX) {
-                    numbytes = APR_DWORD_MAX;
-                }
-                else {
-                    numbytes = (DWORD)bytesleft;
-                }
-
-                if (!WriteFile(thefile->filehand, buffer, numbytes, &written, NULL)) {
-                    rc = apr_get_os_error();
-                    thefile->filePtr += written;
-                    break;
-                }
-
-                thefile->filePtr += written;
-                bytesleft -= written;
-                buffer += written;
+            apr_size_t written;
 
-            } while (bytesleft > 0);
+            rc = write_helper(thefile->filehand, thefile->buffer,
+                              thefile->bufpos, &written);
+            thefile->filePtr += written;
 
             if (rc == 0)
                 thefile->bufpos = 0;
diff --git a/test/testfile.c b/test/testfile.c
index 9087b04f8..692085c80 100644
--- a/test/testfile.c
+++ b/test/testfile.c
@@ -1542,6 +1542,188 @@ static void test_append(abts_case *tc, void *data)
     apr_file_close(f2);
 }
 
+static void test_large_write_buffered(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testlarge_write_buffered.dat";
+    apr_size_t len;
+    apr_size_t bytes_written;
+    apr_size_t bytes_read;
+    char *buf;
+    char *buf2;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_FOPEN_BUFFERED,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for writing", rv);
+
+    /* Test a single large write. */
+    len = 80000;
+    buf = apr_palloc(p, len);
+    memset(buf, 'a', len);
+    rv = apr_file_write_full(f, buf, len, &bytes_written);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    ABTS_INT_EQUAL(tc, (int)len, (int)bytes_written);
+    apr_file_close(f);
+
+    rv = apr_file_open(&f, fname, APR_FOPEN_READ,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for reading", rv);
+
+    buf2 = apr_palloc(p, len + 1);
+    rv = apr_file_read_full(f, buf2, len + 1, &bytes_read);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    ABTS_INT_EQUAL(tc, (int)len, (int)bytes_read);
+    ABTS_TRUE(tc, memcmp(buf, buf2, len) == 0);
+    apr_file_close(f);
+
+    apr_file_remove(fname, p);
+}
+
+static void test_two_large_writes_buffered(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testtwo_large_writes_buffered.dat";
+    apr_size_t len;
+    apr_size_t bytes_written;
+    apr_size_t bytes_read;
+    char *buf;
+    char *buf2;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_FOPEN_BUFFERED,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for writing", rv);
+
+    /* Test two consecutive large writes. */
+    len = 80000;
+    buf = apr_palloc(p, len);
+    memset(buf, 'a', len);
+
+    rv = apr_file_write_full(f, buf, len / 2, &bytes_written);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    ABTS_INT_EQUAL(tc, (int)(len / 2), (int)bytes_written);
+
+    rv = apr_file_write_full(f, buf, len / 2, &bytes_written);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    ABTS_INT_EQUAL(tc, (int)(len / 2), (int)bytes_written);
+
+    apr_file_close(f);
+
+    rv = apr_file_open(&f, fname, APR_FOPEN_READ,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for reading", rv);
+
+    buf2 = apr_palloc(p, len + 1);
+    rv = apr_file_read_full(f, buf2, len + 1, &bytes_read);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    ABTS_INT_EQUAL(tc, (int) len, (int)bytes_read);
+    ABTS_TRUE(tc, memcmp(buf, buf2, len) == 0);
+    apr_file_close(f);
+
+    apr_file_remove(fname, p);
+}
+
+static void test_small_and_large_writes_buffered(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testtwo_large_writes_buffered.dat";
+    apr_size_t len;
+    apr_size_t bytes_written;
+    apr_size_t bytes_read;
+    char *buf;
+    char *buf2;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_FOPEN_BUFFERED,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for writing", rv);
+
+    /* Test small write followed by a large write. */
+    len = 80000;
+    buf = apr_palloc(p, len);
+    memset(buf, 'a', len);
+
+    rv = apr_file_write_full(f, buf, 5, &bytes_written);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    ABTS_INT_EQUAL(tc, 5, (int)bytes_written);
+
+    rv = apr_file_write_full(f, buf, len - 5, &bytes_written);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    ABTS_INT_EQUAL(tc, (int)(len - 5), (int)bytes_written);
+
+    apr_file_close(f);
+
+    rv = apr_file_open(&f, fname, APR_FOPEN_READ,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for reading", rv);
+
+    buf2 = apr_palloc(p, len + 1);
+    rv = apr_file_read_full(f, buf2, len + 1, &bytes_read);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    ABTS_INT_EQUAL(tc, (int) len, (int)bytes_read);
+    ABTS_TRUE(tc, memcmp(buf, buf2, len) == 0);
+    apr_file_close(f);
+
+    apr_file_remove(fname, p);
+}
+
+static void test_write_buffered_spanning_over_bufsize(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testwrite_buffered_spanning_over_bufsize.dat";
+    apr_size_t len;
+    apr_size_t bytes_written;
+    apr_size_t bytes_read;
+    char *buf;
+    char *buf2;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_FOPEN_BUFFERED,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for writing", rv);
+
+    /* Test three writes than span over the default buffer size. */
+    len = APR_BUFFERSIZE + 1;
+    buf = apr_palloc(p, len);
+    memset(buf, 'a', len);
+
+    rv = apr_file_write_full(f, buf, APR_BUFFERSIZE - 1, &bytes_written);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    ABTS_INT_EQUAL(tc, APR_BUFFERSIZE - 1, (int)bytes_written);
+
+    rv = apr_file_write_full(f, buf, 2, &bytes_written);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    ABTS_INT_EQUAL(tc, 2, (int)bytes_written);
+
+    apr_file_close(f);
+
+    rv = apr_file_open(&f, fname, APR_FOPEN_READ,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file for reading", rv);
+
+    buf2 = apr_palloc(p, len + 1);
+    rv = apr_file_read_full(f, buf2, len + 1, &bytes_read);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    ABTS_INT_EQUAL(tc, (int)len, (int)bytes_read);
+    ABTS_TRUE(tc, memcmp(buf, buf2, len) == 0);
+    apr_file_close(f);
+
+    apr_file_remove(fname, p);
+}
+
 #if APR_HAS_THREADS
 typedef struct thread_file_append_ctx_t {
     apr_pool_t *pool;
@@ -1811,6 +1993,10 @@ abts_suite *testfile(abts_suite *suite)
     abts_run_test(suite, test_buffer_set_get, NULL);
     abts_run_test(suite, test_xthread, NULL);
     abts_run_test(suite, test_append, NULL);
+    abts_run_test(suite, test_large_write_buffered, NULL);
+    abts_run_test(suite, test_two_large_writes_buffered, NULL);
+    abts_run_test(suite, test_small_and_large_writes_buffered, NULL);
+    abts_run_test(suite, test_write_buffered_spanning_over_bufsize, NULL);
     abts_run_test(suite, test_datasync_on_file, NULL);
     abts_run_test(suite, test_datasync_on_stream, NULL);
     abts_run_test(suite, test_atomic_append, NULL);