Re: [PATCH] No runtime fetching of built-in global constants

I would propose the attached patch for this optimization. Opcode caches and encoders will have to disable this optimization with ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION Any objections? Thanks. Dmitry. Matt Wilmas wrote: > 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 >