Req #47660 [Asn->Csd]: open/close can be reduced for require_once in ZEND_INCLUDE_OR_EVAL handlers
Edit report at https://bugs.php.net/bug.php?id=47660&edit=1
ID: 47660
Updated by: dmitry@php.net
Reported by: basant dot kukreja at gmail dot com
Summary: open/close can be reduced for require_once in
ZEND_INCLUDE_OR_EVAL handlers
-Status: Assigned
+Status: Closed
Type: Feature/Change Request
Package: Scripting Engine problem
Operating System: *
PHP Version: 5.2.9
Assigned To: dmitry
Block user comment: N
Private report: N
New Comment:
No changes required.
Previous Comments:
------------------------------------------------------------------------
[2009-04-08 20:32:17] 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.
------------------------------------------------------------------------
[2009-04-01 10:52:58] 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-03-31 21:56:08] 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-03-31 21:44:27] 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-15 04:03:06] 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.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=47660
--
Edit this bug report at https://bugs.php.net/bug.php?id=47660&edit=1
Thread (8 messages)