# [PATCH] 1 small optimization and 1 cleanup

- Thread: https://externals.io/message/26945
- Original archive: https://news-web.php.net/php.internals/26945
- Started by Ron Korving on 2006-12-13T18:28:59+00:00
- Messages: 6
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #26945 — Ron Korving — 2006-12-13T18:28:59+00:00

URL: https://externals.io/message/26945#26945

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;
     }

---

## #26955 — Andrei Zmievski — 2006-12-14T16:27:15+00:00

URL: https://externals.io/message/26945#26955 · In reply to: #26945

> +
> + 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

---

## #26956 — Ron Korving — 2006-12-14T17:07:58+00:00

URL: https://externals.io/message/26945#26956 · In reply to: #26955

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 you rewrite it, then please adhere to the code style that was used,
> i.e. with explicit braces for if() statements.
>
> -Andrei

---

## #26957 — Andrei Zmievski — 2006-12-14T17:16:53+00:00

URL: https://externals.io/message/26945#26957 · In reply to: #26956

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...
> > […]

---

## #26959 — Nuno Lopes — 2006-12-14T17:21:56+00:00

URL: https://externals.io/message/26945#26959 · In reply to: #26945

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

---

## #26961 — Ron Korving — 2006-12-14T20:21:52+00:00

URL: https://externals.io/message/26945#26961 · In reply to: #26959

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...
> > […]
saving
> > […]
from
> > […]
>
> 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..
>
>
> > […]