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;
- 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
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...
- 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
Sure.
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...
- 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
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..
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;
}
/* }}} */
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
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..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;
}
/* }}} */