Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:9531
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Sat, 24 Apr 2004 01:49:20 -0700 (Pacific Standard Time)
To: Alan Knowles <alan@akbkhome.com>
cc: internals@lists.php.net
In-Reply-To: <408A1C50.6050805@akbkhome.com>
Message-ID: <Pine.WNT.4.56.0404240133580.208@DELL>
References: <4089E106.4040409@akbkhome.com> <Pine.WNT.4.56.0404232147480.1396@DELL>
 <408A1C50.6050805@akbkhome.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Subject: Re: [PHP-DEV] Performance Consideration of 1 class per file
From: rasmus@php.net (Rasmus Lerdorf)

On Sat, 24 Apr 2004, Alan Knowles wrote:
> Thanks - I just had a look through
> zend_comple.c:zend_include_or_eval_handler
>
> it appears that zend_stream_open .. and hence fopen is called on every
> require_once/include_once
>
> would it make sense to add something as simple as
>
> if (opline->op2.....lval == ZEND_REQUIRE_ONCE) {
>     locations = get_potential_locations(file,PG('include_path'));
>     foreach (locations as location)
>        if (zend_hash_find(&EG(included_files), location)) {
>            // i've already got it !
>            NEXT_OPCODE();
>        }
>     }
> }

I am not sure this will help all that much.  You are taking a hit on the
common case in order to optimize what I believe to be the less common case
of a *_once including a file already included.  For PEAR it might be
common, but outside of PEAR I don't think it is.  We still need to do the
realpath stats prior to this and for the common codepath the fopen still
needs to be done after.  Then again, your check may be fast enough that it
could be worth adding to speed up the failure case.

Overall I think you would get more bang for the buck by coming up with an
intelligent caching stat/realpath implementation.  The obvious drawback
would be that symlinks and other filesystem changes done while the server
is running would not be seen until a restart.

-Rasmus