Hi Dmitry, ----- Original Message ----- From: "Dmitry Stogov" Sent: Thursday, July 24, 2008
> Hi Matt, > > Sorry if I missed it. No problem. :-) > Does this patch make any performance difference? > > I assume it saves on hash lookup during compilation and its really > insignificant time. However it add new scanner rules which may slowdown > the whole scanner. > For now I don't see a big reason, but may be I didn't see something. Yes, I tried to explain the differences in the original message (below). In runtime, FETCH_CONSTANT is saved for the "built-in, global constants" (CONST_PERSISTENT). During compilation, no hash lookup is needed for TRUE/FALSE/NULL since they are caught by the scanner, as you saw. The compile-time hash lookups were added when you eliminated runtime fetching for TRUE/FALSE/NULL a couple years ago, which has since been looking up the other built-in constants too and discarding them, so I just use them. :-) Also, right now if the constant isn't found (zend_get_ct_const()), there's lowercasing and a second lookup -- only for catching case-insensitive TRUE/FALSE/NULL! One thing I was wondering about is if this would cause a problem for opcode caches if they need to know it's really a "constant constant" and not a "literal constant." If so, can probably have a compiler_options flag to disable my compile-time substitution, and the opcode cache can do what it wants (substitution with own optimizer, etc.). > Do you have any other "postponed" patches? > I remember something about string optimizations and long multiply. Yeah. :-) The multiply long one [1] is a pretty small thing that probably should be reviewed for a decision (MM's safe_address() function especially), though it does speed up mul_function() (* operator) on more platforms. The "string changes/optimizations" patch [2] is mostly fine, I think. Just wondering if it's OK to remove the INIT_STRING opcode. BTW, it has changes that make the scanner simpler/smaller if you're concerned about the constants patch adding a few rules. ;-D [1] http://marc.info/?l=php-internals&m=121630449331340&w=2 [2] http://marc.info/?l=php-internals&m=121569400218314&w=2 > Thanks. Dmitry. > Thanks, Matt > Matt Wilmas wrote: > > Hi all, > > > > Never heard anything about this optimization after sending it 3 months ago > > (should've sent a follow-up sooner)... > > > > Is this something that can be done? Dmitry? Details in original message. > > Patch is unchanged, I just updated them for the current file versions. > > > > http://realplain.com/php/const_ct_optimization.diff > > http://realplain.com/php/const_ct_optimization_5_3.diff > > > > > > Thanks, > > Matt > > > > > > ----- Original Message ----- > > From: "Matt Wilmas" > > Sent: Friday, April 18, 2008 > > > >> Hi all, > >> > >> I changed things so that the many "built-in" constants (CONST_PERSISTENT > >> ones) will be replaced at compile-time, saving the FETCH_CONSTANT opcode, > > if > >> these changes are usable. This was added for TRUE/FALSE/NULL 2 years ago, > >> but seems like it can be done for "lots" of others too. > >> > >> Since the change 2 years ago, other constants have been getting looked up > >> also, but just discarded. And if a constant wasn't found, its name was > >> lowercased and looked up again (for case-insensitive TRUE/FALSE/NULL). > >> Lowercasing has been removed, since case-insensitive constants can't be > > done > >> (guess an exception was made for TRUE/FALSE/NULL :-)), and TRUE/FALSE/NULL > >> get flagged in the scanner (not reserved words, which Marcus did briefly a > >> few years ago), skipping a hash lookup. BTW, to get this compile-time > >> optimization in a namespace, it needs to be prefixed (::CONSTANT). > >> > >> I also removed an unnecessary memcmp() in zend_get_constant() -- old code > >> that was needed a long time ago, it appears. > >> > >> http://realplain.com/php/const_ct_optimization.diff > >> http://realplain.com/php/const_ct_optimization_5_3.diff > >> > >> Thoughts? > >> > >> > >> Thanks, > >> Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
