Don't use chunking for stream writes

We're currently splitting up large writes into 8K size chunks, which
adversely affects I/O performance in some cases. Splitting up writes
doesn't make a lot of sense, as we already must have a backing buffer,
so there is no memory/performance tradeoff to be made here.

This change disables the write chunking at the stream layer, but
retains the current retry loop for partial writes. In particular
network writes will typically only write part of the data for large
writes, so we need to keep the retry loop to preserve backwards
compatibility.

If issues due to this change turn up, chunking should be reintroduced
at lower levels where it is needed to avoid issues for specific streams,
rather than unnecessarily enforcing it for all streams.

diff --git a/ext/standard/tests/file/userstreams_006.phpt b/ext/standard/tests/file/userstreams_006.phpt
index c434f4cc624c70bf09a889ae43c832abf8302f2a..3269ccce3ecd74a2812d6747cd345894b0eed580 100644 (file)
--- a/ext/standard/tests/file/userstreams_006.phpt
+++ b/ext/standard/tests/file/userstreams_006.phpt
@@ -32,6 +32,5 @@ bool(true)
 option: 3, 2, 50
 int(-1)
 int(8192)
-size: 42
-size: 28
+size: 70
 int(70)

diff --git a/ext/standard/tests/streams/set_file_buffer.phpt b/ext/standard/tests/streams/set_file_buffer.phpt
index 79df5a441a83de076f21245bc15ff57b8524475b..5e5051c86ed6038dd0654ea5dd9781aed0c60b70 100644 (file)
--- a/ext/standard/tests/streams/set_file_buffer.phpt
+++ b/ext/standard/tests/streams/set_file_buffer.phpt
@@ -43,5 +43,4 @@ option: %d, %d, %d
 int(%i)
 int(%d)
 size: %d
-size: %d
 int(%d)

diff --git a/ext/standard/tests/streams/stream_set_chunk_size.phpt b/ext/standard/tests/streams/stream_set_chunk_size.phpt
index 851fd2cfcff12decc302cb060034e40d6ad36e3f..9705580902a99fa261628fe83e1c9fe644d7d308 100644 (file)
--- a/ext/standard/tests/streams/stream_set_chunk_size.phpt
+++ b/ext/standard/tests/streams/stream_set_chunk_size.phpt
@@ -34,7 +34,7 @@ echo "should return previous chunk size (8192)\n";
 var_dump(stream_set_chunk_size($f, 1));
 echo "should be read without buffer (\$count == 10000)\n";
 var_dump(strlen(fread($f, 10000)));
-echo "should elicit 3 writes of size 1 and return 3\n";
+echo "should have no effect on writes\n";
 var_dump(fwrite($f, str_repeat('b', 3)));
 
 echo "should return previous chunk size (1)\n";
@@ -45,7 +45,7 @@ echo "should elicit one read of size 100 (chunk size)\n";
 var_dump(strlen(fread($f, 50)));
 echo "should elicit no read because there is sufficient cached data\n";
 var_dump(strlen(fread($f, 50)));
-echo "should elicit 2 writes of size 100 and one of size 50\n";
+echo "should have no effect on writes\n";
 var_dump(strlen(fwrite($f, str_repeat('b', 250))));
 
 echo "\nerror conditions\n";
@@ -58,10 +58,8 @@ int(8192)
 should be read without buffer ($count == 10000)
 read with size: 10000
 int(10000)
-should elicit 3 writes of size 1 and return 3
-write with size: 1
-write with size: 1
-write with size: 1
+should have no effect on writes
+write with size: 3
 int(3)
 should return previous chunk size (1)
 int(1)
@@ -73,10 +71,8 @@ read with size: 100
 int(50)
 should elicit no read because there is sufficient cached data
 int(50)
-should elicit 2 writes of size 100 and one of size 50
-write with size: 100
-write with size: 100
-write with size: 50
+should have no effect on writes
+write with size: 250
 int(3)
 
 error conditions

diff --git a/main/streams/streams.c b/main/streams/streams.c
index 098d1be2f68893970d9b1799057ce228c7dd9591..70b36be256f4b6b864fab885ff047efd2e6bae69 100644 (file)
--- a/main/streams/streams.c
+++ b/main/streams/streams.c
@@ -1105,7 +1105,7 @@ PHPAPI zend_string *php_stream_get_record(php_stream *stream, size_t maxlen, con
 /* Writes a buffer directly to a stream, using multiple of the chunk size */
 static ssize_t _php_stream_write_buffer(php_stream *stream, const char *buf, size_t count)
 {
-       ssize_t didwrite = 0, justwrote;
+       ssize_t didwrite = 0;
 
        /* if we have a seekable stream we need to ensure that data is written at the
         * current stream->position. This means invalidating the read buffer and then
@@ -1116,13 +1116,8 @@ static ssize_t _php_stream_write_buffer(php_stream *stream, const char *buf, siz
                stream->ops->seek(stream, stream->position, SEEK_SET, &stream->position);
        }
 
-
        while (count > 0) {
-               size_t towrite = count;
-               if (towrite > stream->chunk_size)
-                       towrite = stream->chunk_size;
-
-               justwrote = stream->ops->write(stream, buf, towrite);
+               ssize_t justwrote = stream->ops->write(stream, buf, count);
                if (justwrote <= 0) {
                        /* If we already successfully wrote some bytes and a write error occurred
                         * later, report the successfully written bytes. */