Win32: Improve apr_file_write() performance on buffered files by minimizing the amount of WriteFile() syscalls when possible. Previously, writing has been implemented with a loop that keeps copying the data to the internal 4KB buffer and writing this buffer to disk by calling WriteFile(4096). This patch reduces the amount of syscalls in such situations by performing a single WriteFile() call without any buffering, if possible. If some data is already buffered, then the buffer is first filled, flushed and the remaining part of the data is written with a single WriteFile(). * file_io/win32/readwrite.c (write_buffered): Perform all the writing in one or two calls to WriteFile(), depending on whether we have something in the buffer, or not. Return an appropriate number of written bytes to satisfy the apr_file_write() function contract. (apr_file_write): Adjust call to write_buffered(). * test/testfile.c (test_large_write_buffered, test_two_large_writes_buffered, test_write_buffered_spanning_over_bufsize): New tests. (testfile): Run the new tests. Patch by: Evgeny Kotkov Index: file_io/win32/readwrite.c =================================================================== --- file_io/win32/readwrite.c (revision 1805607) +++ file_io/win32/readwrite.c (working copy) @@ -290,10 +290,10 @@ static apr_status_t write_helper(HANDLE filehand, } static apr_status_t write_buffered(apr_file_t *thefile, const char *buf, - apr_size_t len) + apr_size_t len, apr_size_t *pwritten) { - apr_size_t blocksize; - apr_status_t rv; + apr_size_t remaining = len; + apr_size_t buf_free; if (thefile->direction == 0) { /* Position file pointer for writing at the offset we are logically reading from */ @@ -306,20 +306,47 @@ static apr_status_t write_buffered(apr_file_t *the thefile->direction = 1; } - rv = 0; - while (rv == 0 && len > 0) { - if (thefile->bufpos == thefile->bufsize) /* write buffer is full */ - rv = apr_file_flush(thefile); + buf_free = thefile->bufsize - thefile->bufpos; + /* Buffer the incoming data if we already have something in the + * buffer or if this data fully fits into it. */ + if (thefile->bufpos > 0 || remaining <= buf_free) { + apr_size_t blocksize; - blocksize = len > thefile->bufsize - thefile->bufpos ? - thefile->bufsize - thefile->bufpos : len; + if (remaining < buf_free) { + blocksize = remaining; + } + else { + blocksize = buf_free; + } memcpy(thefile->buffer + thefile->bufpos, buf, blocksize); thefile->bufpos += blocksize; buf += blocksize; - len -= blocksize; + remaining -= blocksize; } - return rv; + if (remaining) { + apr_status_t rv; + apr_size_t written; + + /* We still have data to write, flush the internal buffer if needed + * and write the remaining block with a single syscall. */ + if (thefile->bufpos > 0) { + rv = apr_file_flush(thefile); + if (rv) { + *pwritten = len - remaining; + return rv; + } + } + rv = write_helper(thefile->filehand, buf, remaining, &written); + thefile->filePtr += written; + if (rv) { + *pwritten = len - remaining + written; + return rv; + } + } + + *pwritten = len; + return APR_SUCCESS; } APR_DECLARE(apr_status_t) apr_file_write(apr_file_t *thefile, const void *buf, apr_size_t *nbytes) Index: test/testfile.c =================================================================== --- test/testfile.c (revision 1805607) +++ test/testfile.c (working copy) @@ -1446,6 +1446,141 @@ 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_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 two 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) @@ -1495,6 +1630,9 @@ 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_write_buffered_spanning_over_bufsize, NULL); return suite; }