Hi, I've got what I hope is the solution to the problem that erases the performance issue, and a patch to implement it, read on for details.
Stanislav Malyshev wrote: >> Currently, in order for this code to be autoload-compatible, we need to >> use all of the classes: >> >> <?php >> namespace PEAR2::Pyrus::PackageFile::v2Iterator; >> use PEAR2::Pyrus::PackageFile::v2Iterator::File; >> use PEAR2::Pyrus::PackageFile::v2Iterator::FileAttribsFilter; >> use PEAR2::Pyrus::PackageFile::v2Iterator::FileContents; >> ?> > > No, you don't. You just need to use namespace::File etc. where > appropriate. You may do what you did too, but you don't have to. Thanks for this pointer, I hadn't realized one could use namespace:: in a use statement. For those who aren't clear on what Stas is saying, here is the better example: <?php namespace PEAR2::Pyrus::PackageFile::v2Iterator; use namespace::File, namespace::FileAttribsFilter, namespace::FileContents; ?> It is worth noting that the actual code I pulled from Pyrus would be in the PEAR2::Pyrus::PackageFile namespace, so the imports would then become namespace::v2Iterator::***. >> performance slowdown can be 100% removed (with identical >> functionality) via: > > So you traded 3 uses for 3 uses. And on top of that - you also traded > imaginary case of somebody in the future adding internal class named > File (how probable is that? Quite near 0 - by now nobody would add > classes with names like that even in core, let alone in extensions) - > which is easily avoidable by using namespace:: in this particular case > and can be thus easily fixed if that improbably thing happens - for > very real case of having to write use for *EVERY* internal class, even > if you *NEVER* intended to override anything with anything but just > using namespaces for your own private stuff. You end up paying for > imaginary case which would never happen in reality with very real > busywork - which is unavoidable and would with 100% guarantee silently > break your application if you forget to do it even once. Another quick note: if namespaces become robust enough, PHP core could introduce any classname we wish without fear of breaking userspace code - which is one of the primary goals of namespacing, is it not? As for the performance nightmare scenario, I decided to whip up a little benchmark: <?php namespace foo; function __autoload($class) { if ($class == 'foo::XMLReader') { class XMLReader { function __construct() {echo __CLASS__,"\n";} } return; } // add performance intensive work to demonstrate performance issue for($i=1;$i<=1000;stat(__FILE__),$i++); } spl_autoload_register(__NAMESPACE__ . '::__autoload'); for ($i=1;$i < 100;$i++) { $a = new Exception('hi'); echo get_class($a),"\n"; } $b = new XMLReader; ?> output: 100x "Exception" followed by "foo::XMLReader" This little script demonstrates an autoload that is about 500x-800x slower than most autoloads, as in most cases, there is a 1=>1 class->filename relationship, and thus only a couple of stat calls are performed per autoload. However, this does demonstrate that in an extreme environment, the difference between having a "use ::Exception;" at the top and the above code is 466x. Yes, that's right, 466 times slower. On my machine, that's a difference of instantaneous versus 3 seconds to execute. Now for the good news. The reason it is so slow is that the autoload is called for every single invocation of the class name. With the patch at http://pear.php.net/~greg/resolution_fix.patch.txt this performance is improved to only 5.2x slower than its counterpart, even with the exceptionally ridiculous autoload. On my machine, there is no perceptible difference in performance to the naked eye. It's a HUGE performance gain over the pre-optimized patch. Note that the way I calculated the performance was by running callgrind against the script, and then doing simple division of the total number of "events" that callgrind reported. Thus, the fastest (with use ::Exception) was 15 million events, the slowest (pre-optimization) was 7.3 billion events, and the optimized was 82 million events, reflecting the additional 65 million events added by the heavy __autoload and the light hash table lookups. It is also important to note that this slowdown would only occur in situations where an unqualified classname is used multiple times in the same file, which most commonly happens with static class methods or class constants. The iterator constants are a good example, as are XMLReader and many other commonly used internal classes. For projects that only use 2-3 internal class names, this patch won't help, but at the same time, the performance issue is muted by having only a few classname references as well. The patch also implements the new classname resolution *and* all existing namespace tests pass. The example code above should become ns_070.phpt. The only caveat - I ran out of time, this patch is for PHP_5_3, and will need the _u_ porting to HEAD for unicode. Now that the performance hit is minimal, can we agree to get along on this proposed change Stas? ;) It transforms the critical nature of the problem to a truly minor performance hit on internal class usage without use or ::. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php