From: Nikita Popov Date: Thu, 18 Jul 2019 15:02:29 +0000 (+0200) Subject: Don't use chunking for stream writes X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5cbe5a538c92d7d515b0270625e2f705a1c02b18;p=php 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 c434f4cc62..3269ccce3e 100644 --- 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 79df5a441a..5e5051c86e 100644 --- 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 851fd2cfcf..9705580902 100644 --- 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 098d1be2f6..70b36be256 100644 --- 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. */