Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:24342
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <00ec01c6a518$7b442720$6400a8c0@binarysecfb111>
To: "Andi Gutmans" <andi@zend.com>,
	"'Dmitry Stogov'" <dmitry@zend.com>,
	"'PHPdev'" <internals@lists.php.net>
Date: Tue, 11 Jul 2006 20:33:17 +0200
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=response
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] FW: Help needed in benchmarking memory patch
From: mv-php@binarysec.com ("Michael Vergoz")

Here is a small patch allowing to correct the small leak. This one will have 
to be merge with that of Michael - (its patch is correct and does not 
contain any leak (I were not clear in my last mail) - thus +1)

http://badcode.be/~descript/PHP/PHP_5_2-php-main.diff

Michael Vergoz

----- Original Message ----- 
From: "Michael Vergoz" <mv-php@binarysec.com>
To: "Andi Gutmans" <andi@zend.com>; "'Dmitry Stogov'" <dmitry@zend.com>; 
"'PHPdev'" <internals@lists.php.net>
Sent: Tuesday, July 11, 2006 8:29 PM
Subject: Re: [PHP-DEV] FW: Help needed in benchmarking memory patch


> Salut Andi & Dmitry,
>
> If you have errors in your script you have a leak in 
> PG(last_error_message) & PG(last_error_file) because of php_error_cb() 
> which allocates via strdup() these two variables. It should be noted also 
> that these variables are not given with NULL after their freed in 
> php_error_cb() - main/main.c +679 and that both last allowance will never 
> be freed for each thread.
> They are freed with the stop of the process via php_module_shutdown().
>
> I propose to free these buffer in php_request_shutdown()? In fact the leak 
> more are the small ones but they become repetitive in a Threaded program.
>
> There is also another condition to generate large mem leak for example by 
> re-using a thread to carry out several script with the continuation. There 
> you explodes all the records if you do not do a ts_free_thread() just 
> after your php_request_shutdown(). It's for that which I think that it 
> would be very judicious to add a ts_free_thread() before each the return() 
> of the HttpExtensionProc() function of the ISAPI SAPI module.
>
> Michael Vergoz
>
> ----- Original Message ----- 
> From: "Andi Gutmans" <andi@zend.com>
> To: "'Michael Vergoz'" <mv-php@binarysec.com>; "'Dmitry Stogov'" 
> <dmitry@zend.com>; "'PHPdev'" <internals@lists.php.net>
> Sent: Tuesday, July 11, 2006 7:38 PM
> Subject: RE: [PHP-DEV] FW: Help needed in benchmarking memory patch
>
>
>> Are you sure this isn't because the IIS thread is still running? One 
>> thread
>> handles multiple requests.
>>
>>> -----Original Message-----
>>> From: Michael Vergoz [mailto:mv-php@binarysec.com]
>>> Sent: Tuesday, July 11, 2006 10:33 AM
>>> To: Dmitry Stogov; 'Andi Gutmans'; PHPdev
>>> Subject: Re: [PHP-DEV] FW: Help needed in benchmarking memory patch
>>>
>>> In fact I had found some leaks it would seem that
>>> ts_free_thread() after
>>> php_request_shutdown() solves all the problem.
>>> I am checking if it's bad but if not the case the SAPI ISAPI
>>> should have a small modification.
>>>
>>> mv-
>>> ----- Original Message -----
>>> From: "Dmitry Stogov" <dmitry@zend.com>
>>> To: "'Michael Vergoz'" <mv-php@binarysec.com>; "'Andi Gutmans'"
>>> <andi@zend.com>; <internals@lists.php.net>
>>> Sent: Tuesday, July 11, 2006 1:54 PM
>>> Subject: RE: [PHP-DEV] FW: Help needed in benchmarking memory patch
>>>
>>>
>>> > Hi Michael,
>>> >
>>> > The patch was tested in ZTS mode too.
>>> >
>>> > What SAPI you are talking about? (I would like to reproduce
>>> the problem).
>>> >
>>> > Thanks. Dmitry.
>>> >
>>> >> -----Original Message-----
>>> >> From: Michael Vergoz [mailto:mv-php@binarysec.com]
>>> >> Sent: Tuesday, July 11, 2006 1:22 PM
>>> >> To: Andi Gutmans; internals@lists.php.net
>>> >> Subject: Re: [PHP-DEV] FW: Help needed in benchmarking memory patch
>>> >>
>>> >>
>>> >> Hi aAndy
>>> >>
>>> >> > We didn't find a material difference between malloc() and mmap()
>>> >> > though, most probably because malloc() at these sizes
>>> >> actually calls
>>> >> > mmap().
>>> >> You can have good difference with the mremap() of Linux
>>> >> because it directly
>>> >> changes the vector into memory without recopying these data.
>>> >>
>>> >> --
>>> >> Before beginning a benchmark I would like to speak about a
>>> >> serious problem
>>> >> that I have with TSRM/MM.
>>> >>
>>> >> A S1 script carried out by thread T1 will use the same memory
>>> >> capacity as S2
>>> >> script by T2 whereas theoretically separation is obligatory.
>>> >> In addition to
>>> >> the problem of safety, that cause also a problem of
>>> >> management of the load
>>> >> memory of the process father.
>>> >>
>>> >> We define a father process P=while(1);
>>> >>
>>> >> When a process father starts it must initialize tsrm_startup(),
>>> >> sapi_startup() and php_module_startup(). At this time all the
>>> >> additional
>>> >> functions and objects are loaded  and usable by all script
>>> >> (what is normal).
>>> >>
>>> >> With the creation of a thread one associates a TSRMLS to
>>> it. Then one
>>> >> carries out php_request_startup(), php_execute_script() and
>>> >> php_request_shutdown() to entirely enclose PHP/Zend.
>>> >>
>>> >> With the execution of Sx script in Tx: the variables
>>> >> dynamics, static and
>>> >> constant of this one will be stored in AG(head).
>>> >>
>>> >> The problem comes from there. It is impossible to destroy a
>>> >> memory which a
>>> >> script has just used because the process father continues to
>>> >> turn and nor
>>> >> php_request_shutdown(), php_module_shutdown(), sapi_shutdown() and
>>> >> tsrm_shutdown() will not be carried out AND THUS there will
>>> >> be a mem leak
>>> >>
>>> >> I am very annoyed by this problem, it is necessary that I
>>> >> find a possibility
>>> >> to destroy these buffer.
>>> >>
>>> >> Michael Vergoz
>>> >>
>>> >> ----- Original Message ----- 
>>> >> From: "Andi Gutmans" <andi@zend.com>
>>> >> To: "'Michael Vergoz'" <mv-php@binarysec.com>;
>>> >> <internals@lists.php.net>
>>> >> Sent: Tuesday, July 11, 2006 8:26 AM
>>> >> Subject: RE: [PHP-DEV] FW: Help needed in benchmarking memory patch
>>> >>
>>> >>
>>> >> > Sorry I didn't make clear that only the malloc()/free()
>>> part works.
>>> >> > The
>>> >> > mmap() and Win32 Allocation code is commented.
>>> >> > We didn't find a material difference between malloc() and
>>> >> mmap() though,
>>> >> > most probably because malloc() at these sizes actually
>>> calls mmap().
>>> >> > So really most important is for some ppl to run some
>>> >> benchmarks on this.
>>> >> >
>>> >> > Thx.
>>> >> >
>>> >> > Andi
>>> >> >
>>> >> >> -----Original Message-----
>>> >> >> From: Michael Vergoz [mailto:mv-php@binarysec.com]
>>> >> >> Sent: Monday, July 10, 2006 5:25 PM
>>> >> >> To: Andi Gutmans; internals@lists.php.net
>>> >> >> Subject: Re: [PHP-DEV] FW: Help needed in benchmarking
>>> memory patch
>>> >> >>
>>> >> >> Hi Andi,
>>> >> >>
>>> >> >> I appreciated to see this code which I must still
>>> study. Already a
>>> >> >> small remark it is that the system call mremap() is not
>>> >> portable it
>>> >> >> is available only on Linux. Then the size of the segments
>>> >> must always
>>> >> >> "be paginated" - normally your code is good on this level.
>>> >> >> I think that a concept of kernel-memory and user-memory can
>>> >> >> be very interesting to study, especially in threads
>>> >> >> environments - that I use.
>>> >> >>
>>> >> >> I would continue to look at your code but now I will
>>> sleep (04:26
>>> >> >> pm)!
>>> >> >>
>>> >> >> mv-
>>> >> >>
>>> >> >> ----- Original Message -----
>>> >> >> From: "Andi Gutmans" <andi@zend.com>
>>> >> >> To: "'Andi Gutmans'" <andi@zend.com>; <internals@lists.php.net>
>>> >> >> Sent: Monday, July 10, 2006 5:57 PM
>>> >> >> Subject: RE: [PHP-DEV] FW: Help needed in benchmarking
>>> memory patch
>>> >> >>
>>> >> >>
>>> >> >> > No help??? Come on guys. I'm sure some of you can spare a
>>> >> >> few idle cycles
>>> >> >> > :)
>>> >> >> >
>>> >> >> >
>>> >> >> >> -----Original Message-----
>>> >> >> >> From: Andi Gutmans [mailto:andi@zend.com]
>>> >> >> >> Sent: Friday, July 07, 2006 8:38 PM
>>> >> >> >> To: internals@lists.php.net
>>> >> >> >> Subject: [PHP-DEV] FW: Help needed in benchmarking
>>> memory patch
>>> >> >> >>
>>> >> >> >> Sending 3rd time this time without the attachment.
>>> >> >> >> You can get the diff at http://gutmans.org/alloc.zip
>>> >> >> >>
>>> >> >> >>
>>> >> >> >> -----Original Message-----
>>> >> >> >> From: Andi Gutmans [mailto:andi@zend.com]
>>> >> >> >> Sent: Thursday, July 06, 2006 11:06 PM
>>> >> >> >> To: 'internals@lists.php.net'
>>> >> >> >> Subject: Help needed in benchmarking memory patch
>>> >> >> >>
>>> >> >> >> Hi all,
>>> >> >> >>
>>> >> >> >> Attached is a patch we've been working on to improve
>>> the memory
>>> >> >> >> management of PHP. This patch's main advantage is
>>> reducing the
>>> >> >> >> memory footprint of PHP, and therefore allowing it to scale
>>> >> >> >> better. Although at low concurrencies the change seems to be
>>> >> >> >> negligble, while testing at higher concurrencies we saw
>>> >> >> >> significant improvement with this patch, mainly due to lower
>>> >> >> >> memory usage.
>>> >> >> >>
>>> >> >> >> We would very much appreciate if people here tested
>>> >> this patch and
>>> >> >> >> send in their results. It would help us understand if
>>> >> others are
>>> >> >> >> getting consistent results with ours, and also see
>>> >> whether there
>>> >> >> >> are any additional improvements we should make.
>>> >> >> >>
>>> >> >> >> This patch should apply cleanly to the PHP 5.2 CVS
>>> >> tree. There's
>>> >> >> >> not much tuning that needs to be done. It uses
>>> >> >> >> malloc() to allocate large memory blocks. You can
>>> >> control the size
>>> >> >> >> of these blocks by setting the ZEND_MM_SEG_SIZE environment
>>> >> >> >> variable before starting Apache/PHP. You may use K
>>> or M to play
>>> >> >> >> around with the block size. The default we are using is 256KB
>>> >> >> >> which seems to be a good balance.
>>> >> >> >>
>>> >> >> >> Any feedback would be appreciated!
>>> >> >> >>
>>> >> >> >> Andi
>>> >> >> >>
>>> >> >> >> --
>>> >> >> >> PHP Internals - PHP Runtime Development Mailing List To
>>> >> >> >> unsubscribe, visit: http://www.php.net/unsub.php
>>> >> >> >>
>>> >> >> >
>>> >> >> > --
>>> >> >> > PHP Internals - PHP Runtime Development Mailing List
>>> >> >> > To unsubscribe, visit: http://www.php.net/unsub.php
>>> >> >> >
>>> >> >> >
>>> >> >>
>>> >> >
>>> >> >
>>> >>
>>> >> -- 
>>> >> PHP Internals - PHP Runtime Development Mailing List
>>> >> To unsubscribe, visit: http://www.php.net/unsub.php
>>> >>
>>> >>
>>> >
>>> > -- 
>>> > PHP Internals - PHP Runtime Development Mailing List
>>> > To unsubscribe, visit: http://www.php.net/unsub.php
>>> >
>>> >
>>>
>>
>>
>