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

Reply via email to