On Tue, 13 Apr 2010, Dmitry Stogov wrote: > I've published all the patches, their description and performance evaluation > at http://wiki.php.net/rfc/performanceimprovements
I've a few questions and comments: http://wiki.php.net/rfc/performanceimprovements#zend_engine_vm_tuning You're writing that the ZEND_CATCH opcode has to be used with a constant class name, so how would "catch ( self $e )" be represented in op-codes now? Some patches seem to be changing tests; I think we should avoid that and instead create a new test that only runs for the new version, and add a qualifier to the old one (the one that you're now modifying) to *not* run with the new PHP version. 2-empty_hash.diff.txt seems to be ignoring some of our coding guidelines: if(ht->nTableMask) pefree(ht->arBuckets, ht->persistent); should clearly be: if (ht->nTableMask) { pefree(ht->arBuckets, ht->persistent); } Could you please check the space between "if" and "(", and always using braces for all the patches? 3-literals.diff.txt makes the opcode arrays again a bit more complex to disect. Perhaps you could add a bit of documentation describing how all the things work together now? Like which index in whivh fields points to which other structure etc? 4-interned.diff.txt The change in array.c, is that strictly necessary, or just another performance improvement? Things like make me wonder, whether it would work to make this macro part of efree() ? (or create an esfree() (string-free) instead? - efree(Z_STRVAL_P(value)); + if (!IS_INTERNED(Z_STRVAL_P(value))) { + efree(Z_STRVAL_P(value)); + } In the RFC you write "Note: The usage of special data structure (e.g. zend_string) instead of char* throughout PHP would probably be a more consistent solution, but it would be a huge break. We are going to look into this solution in the future.". This would be an API break only, right, very similar to what the Unicode patch did with zstr's? with kind regards, Derick -- http://derickrethans.nl | http://xdebug.org Like Xdebug? Consider a donation: http://xdebug.org/donate.php twitter: @derickr and @xdebug -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
