Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:34861
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 212.55.154.23 as permitted sender)
Message-ID: <247E16805B904E159ACE8E1EC4CF1D5F@pc07654>
To: "Dmitry Stogov" <dmitry@zend.com>,
	"PHP Internals List" <internals@lists.php.net>
References: <4794F6DA.8010400@zend.com>
In-Reply-To: <4794F6DA.8010400@zend.com>
Date: Mon, 21 Jan 2008 23:10:43 -0000
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Segmented argument_stack
From: nlopess@php.net ("Nuno Lopes")

Hi Dmitry,

The patch looks fine. Although it wastes a bit more memory than the current 
implementation, I think it's ok. It also has some memory fragmentation, 
which shouldn't be an issue (we don't have functions with 100 arguments :P).

As a side note, I think the following code could be optimized:

+ while (zend_vm_stack_top(TSRMLS_C) != stack_frame) {
+ zval *stack_zval_p = zend_vm_stack_pop(TSRMLS_C);
+ zval_ptr_dtor(&stack_zval_p);
  }

I think you can simply discard the last n ptrs on the stack. A 
zend_vm_stack_pop(num_elements) would do the trick (or a 
zend_vm_stack_pop_until(stack_frame)). That way the function call cleanup 
would be a little faster.

Thanks,
Nuno


----- Original Message ----- 
From: "Dmitry Stogov" <dmitry@zend.com>
To: "PHP Internals List" <internals@lists.php.net>
Sent: Monday, January 21, 2008 7:47 PM
Subject: [PHP-DEV] Segmented argument_stack


> Hi,
>
> The attached patch (for PHP_5_3) implements the segmented argument_stack
> that has the following advantages:
>
> 1) It fixes #43426 and other crashes which occur because of stack
> reallocation.
>
> 2) The whole stack is never reallocated. So we don't have penalties
> because of the while stack copying (it may be 1MB and more).
>
> 3) CPU stack usage reduced, because execute_data, Ts and CVs now are
> stored in the same argument_stack. As result the probability of CPU
> stack overflow is reduced.
>
> 4) func_num_args(), func_get_agr(), func_get_args(),
> debug_print_backtrace() and exception's backtrace now always able to get
>  arguments. They didn't work before in case of incomplete "arguments
> frame". For example: "foo(1, func_num_args());".
>
> 5) bench.php got about 1% speedup
>
> I'm going to commit the patch into HEAD and PHP_5_3 on Thursday.
> Any objections?
>
> Thanks. Dmitry.