Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [PATCH] 1 small optimization and 1 cleanup »

[PATCH] 1 small optimization and 1 cleanup

php.internals

Ron Korving

19 years ago
Hi, Below are 2 patches for the latest 5.2. The first patch rewrites pcre_get_compiled_regex_ex() in ext/pcre/php_pcre.c from line 417, saving up to 3 comparison statements (?:). The second patch removes a pointless statement (setting a local variable right before a return statement) from json_determine_array_type() in ext/json/json.c on line 92. Please review and commit if valid. Kind regards, Ron Korving Index: ext/pcre/php_pcre.c =================================================================== RCS file: /repository/php-src/ext/pcre/php_pcre.c,v retrieving revision 1.209 diff -u -r1.209 php_pcre.c --- ext/pcre/php_pcre.c 10 Oct 2006 12:43:34 -0000 1.209 +++ ext/pcre/php_pcre.c 13 Dec 2006 18:22:07 -0000 @@ -417,18 +417,21 @@ PHPAPI pcre* pcre_get_compiled_regex_ex(char *regex, pcre_extra **extra, int *preg_options, int *compile_options TSRMLS_DC) { pcre_cache_entry * pce = pcre_get_compiled_regex_cache(regex, strlen(regex) TSRMLS_CC); - - if (extra) { - *extra = pce ? pce->extra : NULL; - } - if (preg_options) { - *preg_options = pce ? pce->preg_options : 0; + + if (pce) + { + if (extra) *extra = pce->extra; + if (preg_options) *preg_options = pce->preg_options; + if (compile_options) *compile_options = pce->compile_options; + return pce->re; } - if (compile_options) { - *compile_options = pce ? pce->compile_options : 0; + else + { + if (extra) *extra = NULL; + if (preg_options) *preg_options = 0; + if (compile_options) *compile_options = 0; + return NULL; } - - return pce ? pce->re : NULL; } /* }}} */ Index: ext/json/json.c =================================================================== RCS file: /repository/php-src/ext/json/json.c,v retrieving revision 1.19 diff -u -r1.19 json.c --- ext/json/json.c 19 Oct 2006 20:24:25 -0000 1.19 +++ ext/json/json.c 13 Dec 2006 18:24:55 -0000 @@ -89,7 +89,6 @@ if (Z_TYPE_PP(val) == IS_ARRAY) { myht = HASH_OF(*val); } else { - myht = Z_OBJPROP_PP(val); return 1; }

Andrei Zmievski

19 years ago
On Dec 13, 2006, at 10:28 AM, Ron Korving wrote:
> + > + if (pce) > + { > + if (extra) *extra = pce->extra; > + if (preg_options) *preg_options = pce->preg_options; > + if (compile_options) *compile_options = pce->compile_options; > + return pce->re; > }
If you rewrite it, then please adhere to the code style that was used, i.e. with explicit braces for if() statements. -Andrei

Ron Korving

19 years ago
I can redo the patch if you want. I do notice the braces aren't always used in if-statements (see preg_get_backref() for a few occasions). Do you want me to repost it with braces? - Ron "Andrei Zmievski" <andrei@gravitonic.com> wrote in message news:2da4c44996eb06fb90d7141e5e5ebc02@gravitonic.com...

Andrei Zmievski

19 years ago
Sure. On Dec 14, 2006, at 9:07 AM, Ron Korving wrote:

Nuno Lopes

19 years ago
> Hi, > > Below are 2 patches for the latest 5.2. The first patch rewrites > pcre_get_compiled_regex_ex() in ext/pcre/php_pcre.c from line 417, saving > up > to 3 comparison statements (?:). The second patch removes a pointless > statement (setting a local variable right before a return statement) from > json_determine_array_type() in ext/json/json.c on line 92.
well about the pcre one I could argument that your patch makes the code bigger and potentially slower :P (due to cache misses) Anyway, I do trust the compiler to do such kind of trivial optimizations..

Ron Korving

19 years ago
If you have good reason to believe it will be slower, then naturaly we should forget about that patch. I don't know much about cache hitrates. The other patch removes bogus code, which should be good I reckon. If you still want the patch for PCRE though, a patch with braces follows below. - Ron ### Eclipse Workspace Patch 1.0 #P php5 Index: ext/pcre/php_pcre.c =================================================================== RCS file: /repository/php-src/ext/pcre/php_pcre.c,v retrieving revision 1.209 diff -u -r1.209 php_pcre.c --- ext/pcre/php_pcre.c 10 Oct 2006 12:43:34 -0000 1.209 +++ ext/pcre/php_pcre.c 14 Dec 2006 20:19:38 -0000 @@ -417,18 +417,35 @@ PHPAPI pcre* pcre_get_compiled_regex_ex(char *regex, pcre_extra **extra, int *preg_options, int *compile_options TSRMLS_DC) { pcre_cache_entry * pce = pcre_get_compiled_regex_cache(regex, strlen(regex) TSRMLS_CC); - - if (extra) { - *extra = pce ? pce->extra : NULL; - } - if (preg_options) { - *preg_options = pce ? pce->preg_options : 0; + + if (pce) + { + if (extra) { + *extra = pce->extra; + } + if (preg_options) { + *preg_options = pce->preg_options; + } + if (compile_options) { + *compile_options = pce->compile_options; + } + + return pce->re; } - if (compile_options) { - *compile_options = pce ? pce->compile_options : 0; + else + { + if (extra) { + *extra = NULL; + } + if (preg_options) { + *preg_options = 0; + } + if (compile_options) { + *compile_options = 0; + } + + return NULL; } - - return pce ? pce->re : NULL; } /* }}} */ ""Nuno Lopes"" <nlopess@php.net> wrote in message news:00e501c71fa4$5bdee3a0$0100a8c0@pc07653...
> > Hi, > > > > Below are 2 patches for the latest 5.2. The first patch rewrites > > pcre_get_compiled_regex_ex() in ext/pcre/php_pcre.c from line 417,
saving
> > up > > to 3 comparison statements (?:). The second patch removes a pointless > > statement (setting a local variable right before a return statement)
from