Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:39262
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 209.235.152.149 cause and error)
Message-ID: <00e501c8ed77$3498cf20$0201a8c0@pc1>
To: <internals@lists.php.net>, "Dmitry Stogov" <dmitry@zend.com>
References: <00ea01c8a160$2edd8160$0201a8c0@pc1> <016c01c8eccd$e28cfac0$0201a8c0@pc1> <488835D0.1040005@zend.com>
Date: Thu, 24 Jul 2008 05:22:40 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] No runtime fetching of built-in global constants
From: php_lists@realplain.com ("Matt Wilmas")

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