Re: Comments about PHPTAL package proposal?

Kornel Lesiński wrote: > On 19.04.2009, at 22:15, Greg Beaver wrote: > >> I have not looked at PHPTAL's code directly, but there are definitely >> ways to solve this by requiring users outside of PEAR to use a bootstrap >> file. One very, very important thing is to make certain all of the >> phptal files are in a subdirectory, thus the "config.php" problem simply >> won't exist, it will be "PHPTAL/config.php" (for instance). > > What should this bootstrap file do? > > If you mean: > > set_include_path(dirname(dirname(__FILE__)) . PATH_SEPARATOR . > get_include_path()); > > Then it would break user's code constructed like this: > > set_include_path("./:/pear/"); > require "PHPTAL/PHPTAL.php"; > require "config.php"; > > This code would suddenly include /pear/Config.php instead of ./config.php. ...assuming the user has the Config package installed *and* is running on windows (all reasonable OSes are case-sensitive) > Safer modification of include_path is by appending directories: > > set_include_path(get_include_path() . PATH_SEPARATOR . > dirname(dirname(__FILE__))); > > but this doesn't fix the main problem: > > set_include_path("/pear/:./"); > require "my/custom-phptal/PHPTAL.php"; > > custom-phptal/PHPTAL.php will end up including potentially > incompatible files from /pear/PHPTAL/. but if the user has some customized PHPTAL, they should simply set include_path to "/path/to/my/custom-phptal" before doing the include. I don't see any evidence of a real problem here: everything you mention has been solved by others years ago, and the problems you describe simply don't happen in the real world. The bootstrap file should provide a simple method to load all of PHPTAL's dependent files at once. <?php set_include_path(dirname(__FILE__)); require_once 'PHPTAL.php'; // load other files here restore_include_path(); ?> >> To clarify my point, this is the order of performance without opcode >> cache (as tested by a synthetic benchmark using siege, and verified by >> Stas Malyshev at Zend labs): >> >> 1) include '/full/path/to/file.php'; >> 2) include_once '/full/path/to/file.php'; >> 3) include 'relativepath.php'; >> 4) include_once 'relativepath.php'; >> 5) include CONSTANT . '/relativepath.php'; >> 6) __autoload() with include (no _once) >> 7) include Blah_Loader::Load('ClassName'); > > Even if CONSTANT == '/full/path/'? I think that (assuming no opcode > cache) evaluation of string should still be much faster than extra > disk access involved in relative paths. you would be wrong - the above list is not conjecture, it is based on benchmarking 60 second runs of actual code through apache using siege as a benchmark tool. >> Why is include CONSTANT . '/relativepath.php' slower? The difference is >> not whether APC caches, but when. With a static include statement, APC >> can figure out at compile-time where the file is, and never needs to hit >> the executor in some situations. What this means is that APC can turn >> the opcodes for the include into NOPs, which is much faster than the >> opcodes it takes to replace CONSTANT with its value, concatenate with >> '/relativepath.php' and then pass to the include opcode, which calls >> compile, which invokes APC (finally). > > APC I have (PHP 5.3.0RC2-dev + APC 3.1.2) does not show that behavior. > > I've made a test that would break if include was changed to NOP: > > http://phptal.motion-twin.com/opcodetest/ > > It contains simple includes. No conditional code at all. The trick is > that include_path is modified randomly, and that fact is impossible to > find out at compile time (so even if APC performed cross-file static > analysis during compilation, it wouldn't be able to guard against this > trick). > > Yet, this code works properly - include "file.inc" includes different > file each time. All related files are cached (present in > apc_cache_info()) and their hit count increases as expected. > > My conclusion is that APC does not optimize include(), even in cases > that seem simple and completely constant. > > Anyway, even if in some scenarios include() could be replaced with > NOP, that's very tiny optimization. Core functionality of opcode cache > remains unaffected either way (i.e. files are not reparsed). > Feel free to ignore my advice, but it is based upon actual benchmarking, not supposition, and your conclusion is refuted by the source code of APC itself. Greg