ID:               47660
 User updated by:  basant dot kukreja at gmail dot com
 Reported By:      basant dot kukreja at gmail dot com
 Status:           Open
 Bug Type:         Performance problem
 Operating System: Solaris 10
 PHP Version:      5.2.9
 New Comment:

Why do php does so? Reason is obvious. For included_once files, php
engine has
to make sure that files are included only once. A script can include a
file
e.g
include_once "file1.php"
include_once "file2.php"

If file2.php is just a symlink to file1.php, it should not be included
more
than once. So to assure that php engine calles zend_stream_open which
will
eventually find the real path of the file and open the file.

However, if file is an absolute file then virtual_file_ex already finds
the
real path so we can have a performance optimization for absolute
paths.

Here is my suggested patch :

--------------------------------------------------
diff -r f55e0053325c php-5.2.9RC3/Zend/zend_vm_def.h
--- a/php-5.2.9RC3/Zend/zend_vm_def.h   Wed Mar 11 12:43:20 2009 -0700
+++ b/php-5.2.9RC3/Zend/zend_vm_def.h   Sat Mar 14 20:26:27 2009 -0700
@@ -2851,22 +2851,49 @@
                case ZEND_INCLUDE_ONCE:
                case ZEND_REQUIRE_ONCE: {
                                zend_file_handle file_handle;
+                               char* realfilepath = NULL;
 
                                if (IS_ABSOLUTE_PATH(Z_STRVAL_P(inc_filename),
Z_STRLEN_P(inc_filename))) {
                                        cwd_state state;
+                                       int virtfile_res = 0;
        
                                        state.cwd_length = 0;
                                        state.cwd = malloc(1);
                                        state.cwd[0] = 0;
 
-                                       failure_retval = 
(!virtual_file_ex(&state,
Z_STRVAL_P(inc_filename), NULL, 1) &&
+                                       virtfile_res = !virtual_file_ex(&state, 
Z_STRVAL_P(inc_filename),
NULL, 1);
+                                       failure_retval = (virtfile_res &&
                                                
zend_hash_exists(&EG(included_files), state.cwd,
state.cwd_length+1));
+
+                                       if (virtfile_res && !failure_retval && 
(state.cwd[0] != 0)) {
+                                               realfilepath = 
estrdup(state.cwd);
+                                       }
 
                                        free(state.cwd);
                                }
 
                                if (failure_retval) {
                                        /* do nothing */
+                               } else if (realfilepath) {
+                                       /* file is a absolute file name and 
virtual_file_ex succeeded */
+                                       int type =
(Z_LVAL(opline->op2.u.constant)==ZEND_INCLUDE_ONCE?ZEND_INCLUDE:ZEND_REQUIRE);
+                                       file_handle.filename = realfilepath;
+                                       file_handle.free_filename = 0;
+                                       file_handle.type = ZEND_HANDLE_FILENAME;
+                                       file_handle.opened_path = NULL;
+                                       file_handle.handle.fp = NULL;
+
+                                       new_op_array = 
zend_compile_file(&file_handle, type TSRMLS_CC);
+                                       if (!file_handle.opened_path) {
+                                               file_handle.opened_path = 
estrndup(realfilepath,
strlen(realfilepath));
+                                       }
+                                       if 
(zend_hash_add_empty_element(&EG(included_files),
file_handle.opened_path, strlen(file_handle.opened_path)+1)==SUCCESS) {
+                                               
zend_destroy_file_handle(&file_handle TSRMLS_CC);
+                                       } else {
+                                               
zend_file_handle_dtor(&file_handle);
+                                               failure_retval=1;
+                                       }
+                                       efree(realfilepath);
                                } else if (SUCCESS == 
zend_stream_open(Z_STRVAL_P(inc_filename),
&file_handle TSRMLS_CC)) {
 
                                        if (!file_handle.opened_path) {
--------------------------------------------------

As I submitted the test case, it works fine for include_once of
multiple files
whose names are different but their real paths are actually same.


Previous Comments:
------------------------------------------------------------------------

[2009-03-15 03:53:56] basant dot kukreja at gmail dot com

Description:
------------
There are few ways to include a file. include, require, require_once
and include_once.

Whey include_once and require_once is used, file is opened by invoking
zend_stream_open in ZEND_INCLUDE_OR_EVAL_SPEC_xxx_HANDLER functions.

When we use php with apc, apc caches php compiled cache so no
open/close call
happened for compiled files. However for include_once/require_once
files, php
still opens the file by calling zend_stream_open. APC checks if the
file is in
compiled cache, it serves the file. The opened file is simply closed.
Net
effect is that php opens/closes a file without much use.



Reproduce code:
---------------
testinc.php :
<?php

include_once "/opt3/specweb/ecommerce/testinc1.php";
include_once "/opt3/specweb/ecommerce/testinc2.php";
print "Test from testinc.php\n";
?>

testinc1.php
$ cat /opt3/specweb/ecommerce/testinc1.php
<?php
print "Test from testinc1.php\n";
?>

testinc2.php is just a symlink to testinc1.php.



Expected result:
----------------
I have found that in ecommerce php benchmark, around 1.5% of the
total time is spent for these open/close calls which effectively
doesn't do much. If php doesn't use APC, then this optimization doesn't
make any sense. However APC, alone can not do this optimization.

Here is the dtrace stack trace of open calls.
$ x=$pid; sudo dtrace -n "syscall::*open:entry /pid == $x"' / {
ustack(); printf("arg0 = %s\n", copyinstr(arg0)); }'
dtrace: description 'syscall::*open:entry ' matched 1 probe
CPU     ID                    FUNCTION:NAME
  1   1458                       open:entry
              libc.so.1`__open+0x4
              libc.so.1`open+0x64
              libphp5.so`php_execute_script+0x1b0
              libphp5.so`php5_execute+0x954
             
libns-httpd40.so`__1cbAfunc_native_pool_wait_work6FpGpnGpblock_pnHSession_pnHRequest__iI135_i_+0x19c
             
libns-httpd40.so`__1cO_func_exec_str6FpnKFuncStruct_pnGpblock_pnHSession_pnHRequest__i_+0x74
              libns-httpd40.so`func_exec_str+0x114
              libns-httpd40.so`INTfunc_exec_directive+0x328
              libns-httpd40.so`INTservact_service+0x60c
              libns-httpd40.so`INTservact_handle_processed+0xa8
             
libns-httpd40.so`__1cLHttpRequestUUnacceleratedRespond6M_v_+0xad8
             
libns-httpd40.so`__1cLHttpRequestNHandleRequest6MpnGnetbuf_I_i_+0x68c
              libns-httpd40.so`__1cNDaemonSessionDrun6M_v_+0x254
              libnsprwrap.so`__1cGThreadErun_6M_v_+0x28
              libnsprwrap.so`ThreadMain+0x14
              libnspr4.so`_pt_root+0x1bc
              libc.so.1`_lwp_start
arg0 = .

  1   1458                       open:entry
              libc.so.1`__open+0x4
              libc.so.1`open+0x64
              libphp5.so`_php_stream_fopen+0x1c0
              libphp5.so`_php_stream_fopen_with_path+0x290
              libphp5.so`php_plain_files_stream_opener+0xcc
              libphp5.so`_php_stream_open_wrapper_ex+0x1e8
              libphp5.so`php_stream_open_for_zend_ex+0x60
              libphp5.so`php_stream_open_for_zend+0x28
              libphp5.so`zend_stream_open+0x6c
              libphp5.so`ZEND_INCLUDE_OR_EVAL_SPEC_CONST_HANDLER+0x204
              libphp5.so`execute+0x3e8
              libphp5.so`zend_execute_scripts+0x288
              libphp5.so`php_execute_script+0x4cc
              libphp5.so`php5_execute+0x954
             
libns-httpd40.so`__1cbAfunc_native_pool_wait_work6FpGpnGpblock_pnHSession_pnHRequest__iI135_i_+0x19c
             
libns-httpd40.so`__1cO_func_exec_str6FpnKFuncStruct_pnGpblock_pnHSession_pnHRequest__i_+0x74
              libns-httpd40.so`func_exec_str+0x114
              libns-httpd40.so`INTfunc_exec_directive+0x328
              libns-httpd40.so`INTservact_service+0x60c
              libns-httpd40.so`INTservact_handle_processed+0xa8
arg0 = /opt3/specweb/ecommerce/testinc1.php


Actual result:
--------------
Poor performance because of additional open/close calls.



------------------------------------------------------------------------


-- 
Edit this bug report at http://bugs.php.net/?id=47660&edit=1

Reply via email to