[PHP-DEV] open/close calls of include_once/require_once files.

2009-11-12 Thread Basant Kukreja
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=47660thanks=2


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 

Re: [PHP-DEV] open/close calls of include_once/require_once files.

2009-11-12 Thread Rasmus Lerdorf
A couple of notes.

You make it sound like this happens on all includes.  It is only
include_once/require_once that have this problem.  Regular
include/require do not.

This has been addressed in APC by overriding the opcode and providing
our own opcode handler for this case.  See
http://svn.php.net/viewvc/pecl/apc/trunk/apc_zend.c?view=markuppathrev=289331
line 86.

Having said that, it would be preferable, of course, if we didn't need
to swap out that code in APC but I am not sure your solution for solving
this only for absolute path includes is the right way to go here.

-Rasmus

Basant Kukreja wrote:
 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=47660thanks=2
 
 
 


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] open/close calls of include_once/require_once files.

2009-11-12 Thread Basant Kukreja
On Thu, Nov 12, 2009 at 11:54:55AM -0800, Rasmus Lerdorf wrote:
 A couple of notes.
 
 You make it sound like this happens on all includes.  It is only
 include_once/require_once that have this problem.  Regular
 include/require do not.
Sorry for making it confusing. I meant only for include_once/require_once
files.

 This has been addressed in APC by overriding the opcode and providing
 our own opcode handler for this case.  See
 http://svn.php.net/viewvc/pecl/apc/trunk/apc_zend.c?view=markuppathrev=289331
 line 86.
Thanks for the link.


 Having said that, it would be preferable, of course, if we didn't need
 to swap out that code in APC but I am not sure your solution for solving
 this only for absolute path includes is the right way to go here.
I don't think it is possible to avoid calling zend_stream_open for files other
than absolute paths in PHP.

Rationale : Opcode cacher doesn't need to override the op if php engine can
perform the check with ease. Cacher can however override it if included file is
not in file-system.

Regards,
Basant.



-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php