Edit report at https://bugs.php.net/bug.php?id=47660&edit=1
ID: 47660 Updated by: dmi...@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] dmi...@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] ras...@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