Everything

php

Home » Project » Newsgroups » php.internals » limit for mmaping for readfile (Bug #19749) »

limit for mmaping for readfile (Bug #19749)

php.internals

Wez Furlong

22 years ago

http://bugs.php.net/bug.php?id=19749

It's been playing on my mind recently that this bug
hasn't been addressed; the problem was how to guess
precisely what limit to set for mmaping when doing
something like readfile().
[note that some comments are missing from the bugdb
for that bug]

Well, we can solve the problem by allowing the
sysadmin to decide, as you can see in my patch
against PHP 5:

http://www.php.net/~wez/mmap-limit.diff

The default limit is 8MB. If the limit is set to 0,
no limit is applied.

This is separate from memory_limit, so PHP could
potentially use double the default memory amount for a
short period of time.  This is still better than allowing
it to eat all the RAM when someone decides to readfile()
large media files for example.

--Wez.

Andi Gutmans

22 years ago

Wez,

I have played with mmap() in the past. I found that mapping in relatively
small blocks such as 1MB does not really hurt performance. I suggest not to
add yet another INI option but to do it with smaller blocks. I think even
256KB wouldn't make a big difference.

Andi

At 10:10 PM 4/20/2004 +0100, Wez Furlong wrote:

Wez Furlong

22 years ago

Andi,

Do you think mmap'ing a series of smaller chunks is going to be
faster than mmap'ing the whole file, or faster than reading
chunks conventionally? (just asking; I've not done any tests)

The mmap code in the passthru function will fall back on reading
chunks in a loop if the mmap failed.  The proposed mmap limit
really just a runaway brake to avoid the box swapping to death
if, say, 5 people start downloading 5 .iso images through a PHP
script using readfile().

My patch is "nice" at this stage in the release because it is a
minimal change that can't hurt anything; tweaking the passthru
code is a bit more worrying :/

--Wez.

----- Original Message -----
From: "Andi Gutmans" <andi@zend.com>
To: "Wez Furlong" <wez@thebrainroom.com>; <internals@lists.php.net>
Sent: Wednesday, April 21, 2004 10:52 AM
Subject: Re: [PHP-DEV] limit for mmaping for readfile (Bug #19749)

> Wez,
>
> I have played with mmap() in the past. I found that mapping in relatively
> small blocks such as 1MB does not really hurt performance. I suggest not

to

Andi Gutmans

22 years ago

At 12:10 PM 4/21/2004 +0100, Wez Furlong wrote:

>Andi,
>
>Do you think mmap'ing a series of smaller chunks is going to be
>faster than mmap'ing the whole file, or faster than reading
>chunks conventionally? (just asking; I've not done any tests)

I'm saying that mmap'ing chunks which are relatively big such as 256KB-1MB,
is not much slower than mmap()'ing the whole file in one go.

>The mmap code in the passthru function will fall back on reading
>chunks in a loop if the mmap failed.  The proposed mmap limit
>really just a runaway brake to avoid the box swapping to death
>if, say, 5 people start downloading 5 .iso images through a PHP
>script using readfile().
>
>My patch is "nice" at this stage in the release because it is a
>minimal change that can't hurt anything; tweaking the passthru
>code is a bit more worrying :/

In that case, I'd put a hardcoded limit after which you fallback to
non-mmap()'ed reading and not add an INI option. Then you can add the
tweaked code when you have time/energy.
Andi

Wez Furlong

22 years ago

Which brings us back to the original problem ;)
How big should the limit be?

--Wez.

Andi Gutmans

22 years ago

For now make it like 4MB. When you fix it to mmap() parts of the file I'd
use 256 or 512KB.

Andi

At 12:38 PM 4/21/2004 +0100, Wez Furlong wrote:

Marcus Börger

22 years ago

Hello Andi,

i still cannot say any reason against a configure option here?

marcus

Wednesday, April 21, 2004, 1:42:00 PM, you wrote:

> For now make it like 4MB. When you fix it to mmap() parts of the file I'd
> use 256 or 512KB.

> Andi

> At 12:38 PM 4/21/2004 +0100, Wez Furlong wrote:
>>Which brings us back to the original problem ;)
>>How big should the limit be?
>>
>>--Wez.
>>
>> > In that case, I'd put a hardcoded limit after which you fallback to
>> > non-mmap()'ed reading and not add an INI option. Then you can add the
>> > tweaked code when you have time/energy.
>> > Andi

--
Best regards,
Marcus                            mailto:helly@php.net

Andi Gutmans

22 years ago

It's not a big deal, but I don't see why we should add a configure option
if it's something which we'll remove the minute Wez has some more spare
time and improves the implementation.

At 01:40 AM 4/22/2004 +0200, Marcus Boerger wrote:

Derick Rethans

22 years ago

On Wed, 21 Apr 2004, Andi Gutmans wrote:

> >My patch is "nice" at this stage in the release because it is a
> >minimal change that can't hurt anything; tweaking the passthru
> >code is a bit more worrying :/
>
> In that case, I'd put a hardcoded limit after which you fallback to
> non-mmap()'ed reading and not add an INI option. Then you can add the
> tweaked code when you have time/energy.

I would go for an INI setting because I can imagine situations were
you'd want to even lower this (hardcoded) 8MB limit to say 1MB before
falling back to non-mmap code.

Derick

Andi Gutmans

22 years ago

At 01:49 PM 4/21/2004 +0200, Derick Rethans wrote:

>On Wed, 21 Apr 2004, Andi Gutmans wrote:
>
> > >My patch is "nice" at this stage in the release because it is a
> > >minimal change that can't hurt anything; tweaking the passthru
> > >code is a bit more worrying :/
> >
> > In that case, I'd put a hardcoded limit after which you fallback to
> > non-mmap()'ed reading and not add an INI option. Then you can add the
> > tweaked code when you have time/energy.
>
>I would go for an INI setting because I can imagine situations were
>you'd want to even lower this (hardcoded) 8MB limit to say 1MB before
>falling back to non-mmap code.

So go for a 1MB limit until we improve the code. I don't think we should
add an INI setting if this is something we will eventually solve in our code.

Andi

Wez Furlong

22 years ago

1MB it is.

--Wez.

----- Original Message -----
From: "Andi Gutmans" <andi@zend.com>
To: "Derick Rethans" <derick@php.net>
Cc: "Wez Furlong" <wez@thebrainroom.com>; <internals@lists.php.net>
Sent: Wednesday, April 21, 2004 12:51 PM
Subject: Re: [PHP-DEV] limit for mmaping for readfile (Bug #19749)

> At 01:49 PM 4/21/2004 +0200, Derick Rethans wrote:
> >On Wed, 21 Apr 2004, Andi Gutmans wrote:
> >
> > > >My patch is "nice" at this stage in the release because it is a
> > > >minimal change that can't hurt anything; tweaking the passthru
> > > >code is a bit more worrying :/
> > >
> > > In that case, I'd put a hardcoded limit after which you fallback to
> > > non-mmap()'ed reading and not add an INI option. Then you can add the
> > > tweaked code when you have time/energy.
> >
> >I would go for an INI setting because I can imagine situations were
> >you'd want to even lower this (hardcoded) 8MB limit to say 1MB before
> >falling back to non-mmap code.
>
> So go for a 1MB limit until we improve the code. I don't think we should
> add an INI setting if this is something we will eventually solve in our

code.

Ilia A.

22 years ago

On April 21, 2004 05:52 am, Andi Gutmans wrote:

> I have played with mmap() in the past. I found that mapping in relatively
> small blocks such as 1MB does not really hurt performance. I suggest not to
> add yet another INI option but to do it with smaller blocks. I think even
> 256KB wouldn't make a big difference.

I whole heartedly agree with Andi about not adding more INI options, but
perhaps we could add a configure time option. This would give admins whose
servers have plenty of ram and/or need to open a slightly larger then 1 meg
files the ability to control this limit.

Ilia

Andi Gutmans

22 years ago

At 08:46 AM 4/21/2004 -0400, Ilia Alshanetsky wrote:

>On April 21, 2004 05:52 am, Andi Gutmans wrote:
> > I have played with mmap() in the past. I found that mapping in relatively
> > small blocks such as 1MB does not really hurt performance. I suggest not to
> > add yet another INI option but to do it with smaller blocks. I think even
> > 256KB wouldn't make a big difference.
>
>I whole heartedly agree with Andi about not adding more INI options, but
>perhaps we could add a configure time option. This would give admins whose
>servers have plenty of ram and/or need to open a slightly larger then 1 meg
>files the ability to control this limit.

I don't see why. 1MB should work OK (it will fallback to regular file
reading if it's above this). Wez will fix his code to work better with
mmap() at a later time. No reason to add options for something which will
be fixed.

Andi