Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ob_start chunk_size 0/1 behave differently than any other value <4096 #3815

Open
kkmuffme opened this issue Sep 30, 2024 · 9 comments
Open

Comments

@kkmuffme
Copy link

Description

https://3v4l.org/fHmH7

The following code:

<?php

function my_cb( $a ) {
    return $a;
}

ob_start( 'my_cb', 0 );
ob_start( 'my_cb', 1 );
ob_start( 'my_cb', 2 );

$status = ob_get_status( true );
var_dump( array_combine( array_column( $status, 'chunk_size' ), array_column( $status, 'buffer_size' ) ) );

Resulted in this output:

array(3) {
  [0]=>
  int(16384)
  [1]=>
  int(16384)
  [2]=>
  int(4096)
}

But I expected this output instead:

array(3) {
  [0]=>
  int(4096)
  [1]=>
  int(4096)
  [2]=>
  int(4096)
}

PHP Version

8.3.x

Operating System

No response

@kkmuffme kkmuffme added bug Documentation contains incorrect information Status: Needs Triage labels Sep 30, 2024
@damianwadley
Copy link
Member

That's right, 0 and 1 do have a special meaning...

https://www.php.net/manual/en/function.ob-start.php

chunk_size
If the optional parameter chunk_size is passed, the buffer will be flushed after any block of code resulting in output that causes the buffer's length to equal or exceed chunk_size. The default value 0 means that all output is buffered until the buffer is turned off. See Buffer Size for more details.

https://www.php.net/manual/en/outcontrol.buffer-size.php

With the exception of "URL-Rewriter", the size of output buffers can be set when the buffer is started. If set to 0, the output buffer is only limited by the memory available to PHP. If set to 1, the buffer is flushed after every block of code producing any non-zero length output.

...that should probably be called out directly in the docs instead of buried in another page.

@damianwadley damianwadley removed bug Documentation contains incorrect information Status: Needs Triage labels Sep 30, 2024
@damianwadley damianwadley transferred this issue from php/php-src Sep 30, 2024
@kkmuffme
Copy link
Author

Let me rephrase:

  • why allocate 4 memory pages on 0/1 (4*4096=16384) but only 1 memory page (4096) on every other value <4096?
    Especially for 1 this doesn't make sense (for 0 you could argue that that's a best practice that most pages fit within that and the buffer size won't need dynamic resizing, however practically this is entirely undocumented)

  • why does it allocate at least chunk_size + 1 byte instead of chunk_size for buffer size? (e.g. chunk_size 8192 results in a buffer size of at least 12288 which is 8192+4096, to avoid wasting memory you'd have to set a chunk_size of 8191)

@cmb69
Copy link
Member

cmb69 commented Oct 1, 2024

why allocate 4 memory pages on 0/1 (4*4096=16384) but only 1 memory page (4096) on every other value <4096?

The default size of the buffer is 16KB, and all buffers are aligned to 4KB. That doesn't sound totally unreasonable to me.

  • why does it allocate at least chunk_size + 1 byte instead of chunk_size for buffer size?

Because either the calculation is wrong for multiples of 4KB, or there is the need for a trailing NUL byte, in which case the alignment (4KB) had been badly chosen. Note, though, that the actual buffer is contained in an output buffer structure, and this won't fit in a 4KB page, regardless of which chunk_size you choose. (And actually, it is somewhat more complex.) Nope, it is not

Relevant code:

https://github.com/php/php-src/blob/f1b41d790d64a3a17c88345eb29e634a69bddef0/main/php_output.h#L83-L97

@cmb69
Copy link
Member

cmb69 commented Oct 1, 2024

See php/php-src#16161.

@kkmuffme
Copy link
Author

kkmuffme commented Oct 3, 2024

why allocate 4 memory pages on 0/1 (4*4096=16384) but only 1 memory page (4096) on every other value <4096?

The default size of the buffer is 16KB

Why is the default size of 16KB used when using chunk size of 1? 1 should behave just like chunk size of 2-4096 and allocate 4KB.

What's the reason that the default buffer size is 16 KB? This seems extremely arbitrary.

@cmb69
Copy link
Member

cmb69 commented Oct 3, 2024

Why is the default size of 16KB used when using chunk size of 1? 1 should behave just like chunk size of 2-4096 and allocate 4KB.

Oh, right; another glitch. I'm going to amend php/php-src#16161.

What's the reason that the default buffer size is 16 KB? This seems extremely arbitrary.

I think it's somewhat arbitrary regardless which value would be chosen.

cmb69 added a commit to cmb69/php-src that referenced this issue Oct 3, 2024
@kkmuffme
Copy link
Author

kkmuffme commented Oct 3, 2024

I think it's somewhat arbitrary regardless which value would be chosen.

It doesn't have to be though. Ideally the default size is equal to 1 memory page - which is 4k/8k depending on the system.
This is what nginx does:

By default, the buffer size is equal to one memory page. This is either 4K or 8K, depending on a platform.

https://nginx.org/en/docs/http/ngx_http_fastcgi_module.html#:~:text=a%20single%20connection.-,By%20default%2C%20the%20buffer%20size%20is%20equal%20to%20one%20memory,8K%2C%20depending%20on%20a%20platform.

Not sure if PHP can detect the memory page size without performance penalty - if it can, I think that should be the default.
Since allocating more memory later on (which happens now already), is mostly performance penalty free.
But pre-allocating sizes that are larger than 1 or 2 memory pages not only wastes memory but also has a performance penalty.

@Girgias
Copy link
Member

Girgias commented Oct 3, 2024

I think it's somewhat arbitrary regardless which value would be chosen.

It doesn't have to be though. Ideally the default size is equal to 1 memory page - which is 4k/8k depending on the system. This is what nginx does:

By default, the buffer size is equal to one memory page. This is either 4K or 8K, depending on a platform.

https://nginx.org/en/docs/http/ngx_http_fastcgi_module.html#:~:text=a%20single%20connection.-,By%20default%2C%20the%20buffer%20size%20is%20equal%20to%20one%20memory,8K%2C%20depending%20on%20a%20platform.

Not sure if PHP can detect the memory page size without performance penalty - if it can, I think that should be the default. Since allocating more memory later on (which happens now already), is mostly performance penalty free. But pre-allocating sizes that are larger than 1 or 2 memory pages not only wastes memory but also has a performance penalty.

This should be a feature request on the php-src repo and not on the doc repo.

@cmb69
Copy link
Member

cmb69 commented Oct 3, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants