Re: clarification on the performance of require_once and APC

Alexey Borzov wrote: > Hi, > > Gregory Beaver wrote: >> I just tracked down Rasmus on IRC and asked him lots of questions about >> require_once and APC performance. I think that all of the questions >> raised in June with reference to require_once were addressed. Here are >> the facts: >> >> * The problem that was brought to light by HTML_QuickForm with APC was >> that some files were being included both conditionally and >> unconditionally. The way APC solved this was simply to not cache the >> files or classes within them, obviously resulting in slower >> performance. An example of this is when an HTML element extends another >> element that is used. The user require_once's a driver, and the other >> driver conditionally includes it. > > Don't quite understand here, can you elaborate? Sure. This code: file1.php <?php require_once 'foo.php'; ?> file1.php <?php $driver = 'foo'; require_once $driver . '.php'; ?> is a simplistic example of an unconditional and conditional inclusion of the same file. The problem was when classes have cross-file inheritance relationships between unconditional and conditional inclusion, sometimes the same file was included in different ways on different iterations, resulting in erroneous caching of the class definition. This led to segmentation faults or worse, random fatal errors due to mis-defined classes. If you'd like, I can poke Gopal to find out more detail, but it is actually unsolvable as long as we have both top-level and conditional require_once. >> * autoload is slower than direct unconditional include because it is >> processed at runtime in the exact way a conditional include is processed >> * There is no way to improve the performance of require_once other than >> to remove it and use require >> >> With these facts in place, it proves that although the performance >> difference is small for most users, using require_once has an >> unavoidable performance hit, and the more calls to require_once, the >> worse the hit, even with APC in its most optimized state. So yes, >> performance of *_once *is* an issue, contrary to earlier assertions >> based on Gopal's blog entries. > > Even if there *is* a performance issue, we should weigh both the > (small?) performance improvement and the huge inconvenience for people > having to do pear -> pear2 move. What exactly is the huge inconvenience? Let's say we want to use MDB2 with the mysql driver. Old way: <?php require_once 'MDB2.php'; $db = MDB2::connect('mysqli://...'); ?> With a clever design, this could be the difference: New way (beginners): <?php require '/path/to/MDB2/allfiles.php'; $db = MDB2::connect('mysqli://...'); ?> New way (intermediate): <?php require 'PEAR2/autoload.php'; function __autoload($class) { return PEAR2_autoload($class); } $db = MDB2::connect('mysqli://...'); ?> New way (advanced): <?php include '/full/path/to/MDB2.php'; include '/full/path/to/MDB2/mysqli.php/; $db = MDB2::connect('mysqli://...'); ?> When I say clever design, I mean doing this kind of check for the driver inside MDB2: <?php (assuming we're in namespace PEAR2 and have imported PEAR2::MDB2::Exception as Exception) if (class_exists('MDB2::mysqli', true)) { return true; } elseif (file_exists('PEAR2/MDB2/mysqli.php', true)) { require 'PEAR2/MDB2/mysqli.php'; return true; } elseif (file_exists(dirname(__FILE__) . '/MDB2/mysqli.php')) { require dirname(__FILE__) . '/MDB2/mysqli.php'; return true; } else { throw new Exception('Cannot locate mysqli driver - have you installed PEAR2_MDB2_mysqli package?'); } ?> It is my mistake (and a large one) that the language of the RFC does not currently allow this kind of design. require should probably be allowed as a fallback for loading dependencies if they don't exist, the purpose of the new design is to provide more flexibility, not to limit it. I will think about better wording, and if you have any suggestions, please let us know. >> The best performance is to be achieved by having a single include >> statement for each needed file in the entire application. > > Better yet performance can be achieved by putting everything into a > single file. We should also consider stripping comments and whitespace > from this file so that parser's job is easier if the user does not use > a bytecode cache. ...which is not possible without munging if a file contains any unconditional top-level "require_once" statements >> For users who simply wish to try out a package, performance is not an >> issue, they just need to have an easy way to include all needed >> files. This should be done on the application level by including >> allfiles.php, >> or by setting up an __autoload(). > > And here I'd like to re-ask one of my questions. Currently people can > run usage examples and tests right after they install the package. > After the proposed changes are implemented will they have to mangle > usage examples to make them run? No, and in fact if they do, then we need to change the proposal. I think one solution might be to relax the replacements restriction for documentation and tests files. The new language would read something like: replacements are forbidden in files that are intended to be used in production environment (role="php", role="data", role="www") but are encouraged in documentation examples and in tests in order to demonstrate proper usage out-of-the-box. In other words, by providing two examples and using an install-as tag, it becomes possible to provide examples for unzip-and-go and for pyrus-installed. [Note: these are spur-of-the-moment examples, I am sure better design could be thought of with more thought and experimentation by others] unzip_example1.php: <?php // if you have not installed this package using pyrus, this is the example for you // we are in docs/PEAR2_MDB2/unzip_example1.php // load needed files require dirname(__FILE__) . '/../../PEAR2/MDB2/allfiles.php'; import PEAR2::MDB2; $db = MDB2::connect('mysqli://...'); ?> pyrus_example1.php <?php // if you installed this package using pyrus, this is the example for you // we are in docs/PEAR2_MDB2/unzip_example1.php set_include_path('@php_dir@'); require 'PEAR2/autoload.php'; function __autoload($class) { return PEAR2_autoload($class); } import PEAR2::MDB2; $db = MDB2::connect('mysqli://...'); ?> Tests can use replacements to determine whether the package is installed or not in a setup file: <?php if ('@data_dir@' == '@'.'data_dir@') { // unzip-and-go } else { // installed by pyrus } ?> As a side note, PHPUnit and company are not capable of testing both unzip-and-go and pyrus-installed methodology because they run all tests in the same request, but phpt tests can easily do this >> Unlike PEAR, PEAR2 should support both models equally well. With PEAR, >> the best performance is physically impossible, and beginners often need >> to require_once several files just to get started. > > The "best performance" is not yet achieved, we should put everything > into a single file, see above. > >> There were several synthetic benchmarks posted over the summer that >> support the relative performance of slow => fast with autoload => >> allfiles.php => custom allfiles.php, there's no need to rehash that >> argument. It is also true that the relative "performance" of ease of >> getting started is include '/path/to/PEAR2/Package/Name/allfiles.php'; >> => set up include_path/__autoload() => custom allfiles.php. In >> addition, creating a customized allfiles.php is relatively easy if >> autoload is possible. Simply use __autoload() and set up a shutdown >> function that var_dumps() get_included_files(). > > See above, being unable to run usage examples without mangling them > sounds like a major degradation in ease of use. > >> In short, after extensive research, I still support the initial proposal >> of removing require_once and adding allfiles.php and a README for each >> package. >> >> In my opinion, the next step is to figure out the best way to make this >> new system robust in the case of a missing file/class and to document >> how it should be used. >> >> I would like each package to have a top-level README.PackageName file, >> so that when it is installed or when it is unzipped, this file is easy >> to find. For instance: >> >> pyrus install PEAR2_PackageName >> PEAR2_PackageName installed into /full/path/to/pear2/ - read >> README.PEAR2_PackageName to get started > > If your target audience is the people who are unable to set up > include_path then we have strong empirical evidence that they aren't > reading the docs that are available now in a *single* place on PEAR's > website. Do you honestly expect them to read those README files > scattered throughout the file system? First of all, these will not be scattered throughout the filesystem, they will be in the base directory. In other words: unzip PEAR2_Package.zip ls README* README.PEAR2_Package This will mean that the php_dir of pyrus will contain a README for each installed or unzipped package. Users who are unaccustomed to PEAR are not by definition stupid just as people accustomed to PEAR are not by definition smarter. They are simply used to an older distribution model that is like so: 1) download .zip or .tar.gz 2) extract 3) look for README or INSTALL file and read them both. As I remember it, include_path is an innovation that was introduced into PHP in order to support PEAR. Most users use direct inclusion or assume that they are simply including files relative to the current directory, without knowing that include_path often contains "." Also, again, the inability to set up include_path has nothing to do with the problem. I marked a bug as bogus just this past week because a user was confused about PEAR and the use of custom configuration files plus include_path (http://pear.php.net/bugs/bug.php?id=11733). Although the user was confused, the behavior of PEAR is equally confusing. I have run up against problems with include_path several times, and each time it took me a while to debug because I'm so used to include_path being correct. Your contention that this proposal is designed to attract stupid people is a straw man argument - no one benefits from attracting stupid people. However, there are plenty of smart PHP developers who are simply not as familiar with include_path because they've never needed to know about it. >> the README would contain instructions on using allfiles, or on using >> __autoload(). This file could be autogenerated at packaging time for >> packages that don't have a customized one, as it would be the same for >> most packages. >> >> Within each file, dependencies, both internal and external, need to be >> clearly documented at the top of the file. Whether this should be done >> with only /** */ or additionally with class_exists() and exceptions or >> some other code-based solution is a decision that needs to be made. I >> would prefer beginning users to see a clear and simple error message if >> they try to directly include a file without a missing dependency. >> Making this possible in a way that does not adversely affect performance >> is important. > > Making users manage dependencies manually rather than automatically > including them when needed doesn't sound like an improvement in "ease > of use" department, either. Again, this is not a necessary requirement of the new proposal. The MDB2 allfiles.php could (for example) try to locate its drivers and include them: <?php // include MDB2 files ... // try to locate drivers if (file_exists(dirname(__FILE__) . '/Driver')) { $a = opendir(dirname(__FILE__) . '/Driver'); while (false !== ($entry = readdir($a))) { if ($entry{0} == '.') continue; $p = pathinfo($entry); if (!isset($p['extension'])) continue; if ($p['extension'] == 'php') require dirname(__FILE__) . '/Driver/' . $entry; } } ?> The same could go for dependencies. Attempting to locate dependencies with a simple class_exists() test prior to inclusion could make a true no-effort allfiles.php. <?php // load dependency if (!class_exists('PEAR2::DepClass')) { require dirname(__FILE__) . '/../DepClass/allfiles.php'; } ?> In fact, the same class_exists() test probably should be used for the hypothetical MDB2 files, so that if the user does include 2 allfiles.php of a package that uses MDB2 and MDB2, there won't be a class conflict error. This would result in: <?php if (!class_exists('PEAR2::MDB2')) { // include MDB2 files } ?> All that the new proposal mandates is that we stop using top-level require_once. The rest is experimental solutions to doing things, and all possibilities should be explored until we find one that makes coding with PEAR2 packages just as easy as coding with PEAR packages. Ease of use will not be an issue when all the smart people like you ( :) ) within PEAR figure out new ways of doing things within the new parameters. One obvious example of a process that would be far simpler without the problem of top-level require_once munging is the go-pear script. Currently, it has to do all kinds of non-obvious weird stuff to get things downloaded, and additionally is very difficult to read. If we remove all the require_once, go-pear could literally use the PEAR installer itself by cramming all of the needed files into a single file, and putting the installation script at the top of the file. This would even allow us to consider not using a phar archive for PEAR installation within PHP, but still supporting a single-file installation model. Thanks, Greg