Bug #77930 [Opn->Csd]: stream_copy_to_stream should use mmap more often
| From: | nikic@php.net | Date: | Thu, 31 Oct 2019 08:37:36 +0000 |
| Subject: | Bug #77930 [Opn->Csd]: stream_copy_to_stream should use mmap more often | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-223528@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=77930&edit=1
ID: 77930
Updated by: nikic@php.net
Reported by: maggus dot staab at googlemail dot com
Summary: stream_copy_to_stream should use mmap more often
-Status: Open
+Status: Closed
Type: Bug
Package: Streams related
PHP Version: 7.2.17
Block user comment: N
Private report: N
New Comment:
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=333d607d47ca6eec96637c1c8686591dec6f5a0b
Log: Fix bug #77930: Remove mmap limit
Previous Comments:
------------------------------------------------------------------------
[2019-10-30 11:58:48] nikic@php.net
After staring at this code for a bit, I think we should just drop the size limitation entirely, for
a couple of reasons:
First, the limitation already doesn't trigger if you copy the whole file (i.e. use copy() or
stream_copy_to_stream() and don't specify a length). This happens because length will be 0 at
the time of the check and only later calculated based on the file size. This means that we're
already completely blowing the length limit for what is is likely the most common case, and it
doesn't seem like anyone complained about that.
Second, the premise of the code comment ("to avoid runaway swapping") seems incorrect to
me. Because this performs a file-backed non-private mmap, no swap backing is needed for the mapping.
Concerns over "memory usage" are also misplaced, as this is a virtual mapping.
------------------------------------------------------------------------
[2019-10-30 11:23:17] nikic@php.net
Is there any script that can be used to benchmark this?
------------------------------------------------------------------------
[2019-04-23 07:35:38] maggus dot staab at googlemail dot com
please find the very detailed bug report on https://github.com/sabre-io/http/pull/119
------------------------------------------------------------------------
[2019-04-23 07:31:50] maggus dot staab at googlemail dot com
Description:
------------
we got a bug report in our http client library https://github.com/sabre-io/http/pull/119 which
suggests to change our call to stream_copy_to_stream() to stream in chunks with a size of 4MiB
because its nearly 2x faster .
the reason is that when you only copy in 4MiB chunks php internally uses mmap.
wouldn't it make sense that stream_copy_to_stream() internally would do the chunking to speedup
this use-case instead (on php-src level)?
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=77930&edit=1