Thanks basant. The updated diff for dtrace probe is now here
http://cr.opensolaris.org/~sn123202/php529.mar10/webrev/usr/src/cmd/php5/patches/php_dtrace.patch.html

and the updated webrev is at
http://cr.opensolaris.org/~sn123202/php529.mar10/webrev/

Note: Special thanks to David Soria Parra Probe for providing these probes

- Sriram

Basant Kumar kukreja wrote:
> In zend_vm_execute.skl (or zend_vm_execute.h) :
> ++      char* classname = get_active_class_name(&scope TSRMLS_CC);
> ++      DTRACE_FUNCTION_ENTRY(get_active_function_name(TSRMLS_C),
> ++          zend_get_executed_filename(TSRMLS_C), 
> zend_get_executed_lineno(TSRMLS_C),
>
> get_active_class_name and get_active_function_name, 
> zend_get_executed_filename,
> and zend_get_executed_lineno are called two times.  I think it is possible to
> reduce it to single call.  It can be saved before DTRACE_FUNCTION_ENTRY and 
> use
> it later.
>
> I think most of the dscripts will use both at the same time so it is better to
> save these return values in local variables.
>
> ------------------------------------------------------
>
> I think it would be better idea (to maintain) if we the following two lines
> after #if PHP_WIN32 block.
>
> ++  DTRACE_REQUEST_SHUTDOWN();
> ++
>
> Regards,
> Basant.
>
>
> On Tue, Mar 10, 2009 at 03:22:29PM -0700, Sriram Natarajan wrote:
>   
>> Nick Kew wrote:
>>     
>>> But I don't think I follow the patch format used here: patch to a patch
>>> in php_dtrace.patch and php_build_config.m4.patch?
>>>
>>>       
>> I see what you meant. Webrev report messed up while looking at the top  
>> level. However, if you click under ??ew' for these patches, you will see  
>> a proper output like below
>> http://cr.opensolaris.org/~sn123202/php529.mar9/webrev/usr/src/cmd/php5/patches/php_dtrace.patch.html
>>
>> thanks
>> sriram
>> _______________________________________________
>>
>>
>> webstack-discuss mailing list
>> webstack-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/webstack-discuss
>>     

Reply via email to