ext/pgsql: convert regex match, using pcre jit if available and enabled. #15039
Conversation
|
Technically this does seem right, but I wonder about the performance: is the extra effort spent JIT compiling worth it for performing a single match? It seems that the regexes can be quite complex, so it's a bit of a shame that they have to be recompiled each time (already true today, but even more true when JITting). |
|
ok will give it a try |
devnexen
force-pushed
the
pgsql_match_usejit
branch
from
9fa7735 to
e45f407
Compare
ext/pgsql/pgsql.c
Outdated
| zend_hash_destroy(&PGG(connections)); | ||
|
|
||
| for (i = 0; i < 11; i ++) |
There was a problem hiding this comment.
You can merge the assignment and declaration of i, we're in C99.
There was a problem hiding this comment.
Can you also use a sizeof expression instead of the hardcoded 11? Or alternatively, use a define instead of 11 for both the declaration of the array and this loop?
ext/pgsql/pgsql.c
Outdated
| php_error_docref(NULL, E_WARNING, "Cannot allocate match data"); | ||
| return FAILURE; | ||
| } | ||
| res = pcre2_match(re, (PCRE2_SPTR)ZSTR_VAL(str), ZSTR_LEN(str), 0, 0, match_data, php_pcre_mctx()); | ||
| centry->refcount ++; |
ext/pgsql/pgsql.c
Outdated
| php_pcre_free_match_data(match_data); | ||
| pcre2_code_free(re); | ||
| centry->refcount --; |
ext/pgsql/pgsql.c
Outdated
| php_error_docref(NULL, E_WARNING, "Cannot allocate match data"); | ||
| return FAILURE; | ||
| } | ||
| res = pcre2_match(re, (PCRE2_SPTR)ZSTR_VAL(str), ZSTR_LEN(str), 0, 0, match_data, php_pcre_mctx()); | ||
| centry->refcount ++; | ||
| res = pcre2_match(centry->re, (PCRE2_SPTR)ZSTR_VAL(str), ZSTR_LEN(str), 0, 0, match_data, php_pcre_mctx()); |
There was a problem hiding this comment.
Use php_pcre_pce_re to obtain re.
There was a problem hiding this comment.
cool I did not notice them.
ext/pgsql/pgsql.c
Outdated
| return FAILURE; | ||
| } | ||
|
|
||
| match_data = php_pcre_create_match_data(0, re); | ||
| match_data = php_pcre_create_match_data(0, centry->re); |
There was a problem hiding this comment.
Use php_pcre_pce_re to obtain re.
ext/pgsql/pgsql.c
Outdated
| @@ -147,6 +147,17 @@ ZEND_TSRMLS_CACHE_DEFINE() | |||
| ZEND_GET_MODULE(pgsql) | |||
| #endif | |||
|
|
|||
| struct _pcre_cache_entry { | |||
There was a problem hiding this comment.
Please don't import internals from another extension into this extension. I added some suggestions on how to avoid this.
ext/pgsql/pgsql.c
Outdated
| memset(pgsql_globals, 0, sizeof(zend_pgsql_globals)); | ||
| zend_hash_init(&pgsql_globals->connections, 0, NULL, NULL, 1); | ||
|
|
||
| #define ADD_REGEX(reg) pgsql_globals->regexes[i ++] = zend_string_init(reg, strlen(reg), true) |
There was a problem hiding this comment.
Can we add a bounds check assertion please?
devnexen
force-pushed
the
pgsql_match_usejit
branch
from
e45f407 to
985b227
Compare
No description provided.