php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #47660 open/close can be reduced for require_once in ZEND_INCLUDE_OR_EVAL handlers
Submitted: 2009-03-15 03:53 UTC Modified: 2011-09-15 09:50 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:0 (0.0%)
From: basant dot kukreja at gmail dot com Assigned: dmitry (profile)
Status: Closed Package: Scripting Engine problem
PHP Version: 5.2.9 OS: *
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: basant dot kukreja at gmail dot com
New email:
PHP Version: OS:

 

 [2009-03-15 03:53 UTC] 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.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2009-03-15 04:03 UTC] basant dot kukreja at gmail dot com
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.
 [2009-03-31 21:44 UTC] basant dot kukreja at gmail dot com
Here is the performance data collected :
For 30 minute 8 core measurement :
Before this patch :
__open 285.019 sec (Inclusive system time):

After this patch :
__open 124.747 sec (Inclusive system time):

So with submitted patch, specweb ecommerce benchmark spent 160 second less
time.
 [2009-03-31 21:56 UTC] rasmus@php.net
How does this handle the case where:

/some/path/to/file.php
/other/full/path/to/file.php

where /other is a symlink to /some ?
 [2009-04-01 10:52 UTC] dmitry@php.net
It looks like the patch doesn't care about symlinks.
Anyway, php-5.3 already has a general way for eliminating open() syscall on require_once().
 [2009-04-08 20:32 UTC] basant dot kukreja at gmail dot com
As I mentioned in my test case, that I tested with symlinks and it worked fine. Meaning it included only once.

I will check php 5.3's open call performance. 

Thanks for looking into this anyway.
 [2011-02-21 20:42 UTC] jani@php.net
-Package: Feature/Change Request +Package: Scripting Engine problem
 [2011-09-15 09:50 UTC] dmitry@php.net
-Status: Assigned +Status: Closed
 [2011-09-15 09:50 UTC] dmitry@php.net
No changes required.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Apr 27 18:01:35 2024 UTC