Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:46034
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.218.213 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=date:from:to:subject:message-id:mime-version:content-type
         :content-disposition:user-agent;
        b=U129RpQaInZocTq384rCEJKNUxxKvTMs4Xe30eOcAHve7684ClQJiGPnrotrT3W//4
         YB1Pf8EhqcIbNqtlEshtTsq9/ZkbT7HKtrFgVBhHnskkScYMIOnBEz03dYBnWbV7HepV
         jXeuuVbvfm3rhBYIeyic2TtWraM++hicYsQjA=
Date: Thu, 12 Nov 2009 10:55:55 -0800
To: internals@lists.php.net
Message-ID: <20091112185555.GB4294@lbasantk3.red.iplanet.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="r5Pyd7+fXNt84Ff3"
Content-Disposition: inline
User-Agent: Mutt/1.5.19 (2009-01-05)
Subject: open/close calls of include_once/require_once files.
From: basant.kukreja@gmail.com (Basant Kukreja)

--r5Pyd7+fXNt84Ff3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi, 
    When php script includes a script,  php engine compiles the included file.
When opcode caching extensions e.g apc are used, re-compilation is avoided by
apc. But there is a performance problem here. Since php code uses
zend_stream_open to open the included file, open/close system call is still
there. When used with APC, open/close doesn't do any good for file-system based
files. It only provides the realpath of the opened file. In php 5.3,
zend_resolve_path already provides the realpath of the file being opened so we
can avoid an open/close calls for file to be included if file is an absolute
path.

Current php 5.3 implementation for include_once/require_once (pseudo code)

a) call zend_resolve_path. It will provide the realpath. Symlinks are taken
care of by zend_resolve_path.
b) Check if file is already included. If yes, do nothing else go to step (c)
c) Call zend_stream_open and get the included_file realpath.
d) Invoke zend_compile_file and do the rest.

Now zend_stream_open invokes the open/close calls to the including files which
is redundant when caches are used.

Here is the suggested performance fix :
a) Same as previous
b) Same as previous
c) If resolved_path is an absolute path then do step c.1 else do c.2
c.1) Prepare a file_handle and directly invoke zend_compile_file
c.2 Call zend_stream_open and get the included_file realpath.
b) Same as previous

---------------------------------------------------

Few notes :
* Performance improvements using this patch = ~1.5% in an e-commerce workload.
* Testing : I ran the php 5.3 test suite and didn't find any regression.
* If files which are included are not absolute paths then they will follow
  the same zend_stream_path route.
* Tested with symlinks and found the behaviour is expected (no change).

I have attached the patch for review comments.

Regards,
Basant.

Note: I previously did this work for php 5.2 but bug remains still open. 5.3
implementation is relatively straight forward. Here is the reference to 5.2 bug
: http://bugs.php.net/bug.php?id=47660&thanks=2



--r5Pyd7+fXNt84Ff3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="include_file_open_php53.patch"

Index: Zend/zend_vm_def.h
===================================================================
--- Zend/zend_vm_def.h	(revision 290447)
+++ Zend/zend_vm_def.h	(working copy)
@@ -3213,16 +3213,38 @@
 		case ZEND_REQUIRE_ONCE: {
 				zend_file_handle file_handle;
 				char *resolved_path;
+				int is_resolved_abs_path = 0;
 
 				resolved_path = zend_resolve_path(Z_STRVAL_P(inc_filename), Z_STRLEN_P(inc_filename) TSRMLS_CC);
 				if (resolved_path) {
-					failure_retval = zend_hash_exists(&EG(included_files), resolved_path, strlen(resolved_path)+1);
+					int len = strlen(resolved_path);
+					failure_retval = zend_hash_exists(&EG(included_files), resolved_path, len+1);
+					if (IS_ABSOLUTE_PATH(resolved_path, len + 1))
+						is_resolved_abs_path = 1;
 				} else {
 					resolved_path = Z_STRVAL_P(inc_filename);
 				}
 
 				if (failure_retval) {
 					/* do nothing, file already included */
+				} else if (is_resolved_abs_path) {
+					/* file is a absolute file name and resolved_path contains the realpath */
+					int type = (Z_LVAL(opline->op2.u.constant) == ZEND_INCLUDE_ONCE ?
+								ZEND_INCLUDE : ZEND_REQUIRE);
+					file_handle.filename = resolved_path;
+					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 = estrdup(resolved_path);
+					}
+					if (zend_hash_add_empty_element(&EG(included_files), file_handle.opened_path, strlen(file_handle.opened_path)+1)!=SUCCESS) {
+						failure_retval=1;
+					}
+					zend_destroy_file_handle(&file_handle TSRMLS_CC);
 				} else if (SUCCESS == zend_stream_open(resolved_path, &file_handle TSRMLS_CC)) {
 
 					if (!file_handle.opened_path) {

--r5Pyd7+fXNt84Ff3--