ID:               47487
 Updated by:       lbarn...@php.net
 Reported By:      basant dot kukreja at gmail dot com
 Status:           Assigned
 Bug Type:         Streams related
 Operating System: Solaris 10
 PHP Version:      5.2.9RC3
 Assigned To:      lbarnaud
 New Comment:

Thanks.

The following patch reverts the performance penalty introduced in the
fix of #44607 :

--- main/streams/streams.c      8 Jan 2009 19:21:25 -0000       1.82.2.6.2.33
+++ main/streams/streams.c      25 Feb 2009 12:09:01 -0000
@@ -597,7 +597,7 @@ PHPAPI size_t _php_stream_read(php_strea
                if (!stream->readfilters.head && (stream->flags &
PHP_STREAM_FLAG_NO_BUFFER || stream->chunk_size == 1)) {
                        toread = stream->ops->read(stream, buf, size TSRMLS_CC);
                } else {
-                       php_stream_fill_read_buffer(stream, size TSRMLS_CC);
+                       php_stream_fill_read_buffer(stream, MIN(size, 
stream->chunk_size)
TSRMLS_CC);
 
                        toread = stream->writepos - stream->readpos;
                        if (toread > size) {


I will do more tests on this and commit after 5.2.9 is out.


Previous Comments:
------------------------------------------------------------------------

[2009-02-24 20:18:37] basant dot kukreja at gmail dot com

Here is the script :

testinc.php :
<?php
print "Hello\n";
file_get_contents("/tmp/" . 'index');
?>

$ mkfile 60k /tmp/index
$ ./sapi/cli/php /tmp/testinc.php 
Hello
php_stream_fill_read_buffer :reallocating 8192
php_stream_fill_read_buffer :reallocating 16384
php_stream_fill_read_buffer :reallocating 24576
php_stream_fill_read_buffer :reallocating 32768
php_stream_fill_read_buffer :reallocating 40960
php_stream_fill_read_buffer :reallocating 49152
php_stream_fill_read_buffer :reallocating 57344

-----------------------------------------------

printfs are coming from my fprintf addition in
php_stream_fill_read_buffer:
    fprintf(stderr, "php_stream_fill_read_buffer :reallocating %d\n", 
            stream->readbuflen);
-----------------------------------------------

------------------------------------------------------------------------

[2009-02-24 17:10:34] lbarn...@php.net

Do you mean that you experience this problem when including files ? I
believe the parser/scanner reads by chunks of 8K. Can you please provide
a reproduce script (with the large file) ?

The fix for #44607 fixed php_stream_fill_read_buffer() so that it fills
the buffer with "size" bytes, as it is expected to do, and as it was
already doing on filtered streams.

If you call fread() with a size of 60K, php_stream_fill_read_buffer()
will return 60K. Before the fix, it returned 8K, and php_stream_read()
had to call it again and to do memcpy()s too.

------------------------------------------------------------------------

[2009-02-24 15:39:10] johan...@php.net

Arnaud, you fixed the other issue, can you please take a look at this
regression. Thanks.

------------------------------------------------------------------------

[2009-02-24 09:10:00] basant dot kukreja at gmail dot com

Sorry, file name was missing from the patch :

--- a/php-5.2.9RC3/main/streams/streams.c       Sun Feb 22 19:57:30
2009 -0800
+++ b/php-5.2.9RC3/main/streams/streams.c       Tue Feb 24 00:50:21
2009 -0800
@@ -1251,6 +1253,11 @@
         * 2K).  */
        if (php_stream_stat(src, &ssbuf) == 0 && ssbuf.sb.st_size > 0)
{
                max_len = ssbuf.sb.st_size + step;
+               if (max_len > src->readbuflen) {
+                       src->readbuflen = max_len;
+                       src->readbuf = perealloc(src->readbuf,
src->readbuflen,
+                                                              
src->is_persistent);
+               }
        } else {
                max_len = step;
        }

------------------------------------------------------------------------

[2009-02-24 08:59:45] basant dot kukreja at gmail dot com

There might be several possible way to fix it. Here is one alternative
which requires more memory but avoid includes :

@@ -1251,6 +1253,11 @@
         * 2K).  */
        if (php_stream_stat(src, &ssbuf) == 0 && ssbuf.sb.st_size > 0)
{
                max_len = ssbuf.sb.st_size + step;
+               if (max_len > src->readbuflen) {
+                       src->readbuflen = max_len;
+                       src->readbuf = perealloc(src->readbuf,
src->readbuflen,
+                                                              
src->is_persistent);
+               }
        } else {
                max_len = step;
        }
--------------------------------
The above patch though have single alloc but still require large
memory allocation. Before this patch too, large memory is allocated
but by several folds.

Before PR http://bugs.php.net/bug.php?id=44607
was fixed, php only required 8KB memory at a time and no reallocs
happened.

------------------------------------------------------------------------

The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
    http://bugs.php.net/47487

-- 
Edit this bug report at http://bugs.php.net/?id=47487&edit=1

Reply via email to