From 07226f474da93747f2228988a3a3ed09374a3ea0 Mon Sep 17 00:00:00 2001 From: kotkov Date: Tue, 7 Feb 2023 10:51:29 +0000 Subject: On 1.7.x branch: Merge r1806299, r1806301, r1806308, r1806610 from trunk: apr_file_write: Optimize large writes to buffered files on Windows. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.7.x@1907499 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 + file_io/win32/readwrite.c | 143 ++++++++++++++++++++++------------- test/testfile.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+), 53 deletions(-) diff --git a/CHANGES b/CHANGES index ba2c253c5..9c30d2899 100644 --- a/CHANGES +++ b/CHANGES @@ -16,6 +16,9 @@ Changes for APR 1.7.3 *) Don't seek to the end when opening files with APR_FOPEN_APPEND on Windows. [Evgeny Kotkov ] + *) apr_file_write: Optimize large writes to buffered files on Windows. + [Evgeny Kotkov ] + Changes for APR 1.7.2 *) Correct a packaging issue in 1.7.1. The contents of the release were diff --git a/file_io/win32/readwrite.c b/file_io/win32/readwrite.c index 7ff5325b0..62a6147d2 100644 --- a/file_io/win32/readwrite.c +++ b/file_io/win32/readwrite.c @@ -246,6 +246,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; @@ -266,38 +351,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); } @@ -598,34 +655,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 d0a930c66..725b2a14e 100644 --- a/test/testfile.c +++ b/test/testfile.c @@ -1687,6 +1687,188 @@ static void test_append_read(abts_case *tc, void *data) apr_file_remove(fname, p); } +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); +} + abts_suite *testfile(abts_suite *suite) { suite = ADD_SUITE(suite) @@ -1741,6 +1923,10 @@ abts_suite *testfile(abts_suite *suite) abts_run_test(suite, test_atomic_append, NULL); abts_run_test(suite, test_append_locked, NULL); abts_run_test(suite, test_append_read, 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); return suite; } -- cgit v1.2.1