On 1/20/22 12:09 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Thu Jan 20 11:09:34 2022
> New Revision: 1897240
>
> URL: http://svn.apache.org/viewvc?rev=1897240&view=rev
> Log:
> ap_regex: Use Thread Local Storage (if efficient) to avoid allocations.
>
> PCRE2 wants an opaque context by providing the API to allocate and free it, so
> to minimize these calls we maintain one opaque context per thread (in Thread
> Local Storage, TLS) grown as needed, and while at it we do the same for PCRE1
> ints vectors. Note that this requires a fast TLS mechanism to be worth it,
> which is the case of apr_thread_data_get/set() from/to apr_thread_current()
> when APR_HAS_THREAD_LOCAL; otherwise we'll do the allocation and freeing for
> each ap_regexec().
>
> The small stack vector is used for PCRE1 && !APR_HAS_THREAD_LOCAL only now.
>
>
> Modified:
> httpd/httpd/trunk/server/main.c
> httpd/httpd/trunk/server/util_pcre.c
>
> Modified: httpd/httpd/trunk/server/util_pcre.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1897240&r1=1897239&r2=1897240&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_pcre.c (original)
> +++ httpd/httpd/trunk/server/util_pcre.c Thu Jan 20 11:09:34 2022
> @@ -53,6 +53,8 @@ POSSIBILITY OF SUCH DAMAGE.
> */
>
> #include "httpd.h"
> +#include "apr_version.h"
Why is this needed?
> +#include "apr_portable.h"
apr_thread_proc.h
is not sufficent?
> #include "apr_strings.h"
> #include "apr_tables.h"
>
> @@ -263,7 +265,122 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t *
> * ints. However, if the number of possible capturing brackets is small, use
> a
> * block of store on the stack, to reduce the use of malloc/free. The
> threshold
> * is in a macro that can be changed at configure time.
> + * Yet more unfortunately, PCRE2 wants an opaque context by providing the API
> + * to allocate and free it, so to minimize these calls we maintain one opaque
> + * context per thread (in Thread Local Storage, TLS) grown as needed, and
> while
> + * at it we do the same for PCRE1 ints vectors. Note that this requires a
> fast
> + * TLS mechanism to be worth it, which is the case of
> apr_thread_data_get/set()
> + * from/to apr_thread_current() when APR_HAS_THREAD_LOCAL; otherwise we'll do
> + * the allocation and freeing for each ap_regexec().
> */
> +
> +#ifdef HAVE_PCRE2
> +typedef pcre2_match_data* match_data_pt;
> +typedef size_t* match_vector_pt;
> +#else
> +typedef int* match_data_pt;
> +typedef int* match_vector_pt;
> +#endif
> +
> +#if APR_HAS_THREAD_LOCAL
> +
> +static match_data_pt get_match_data(apr_size_t size,
> + match_vector_pt *ovector,
> + match_vector_pt small_vector)
> +{
> + apr_thread_t *current;
> + struct {
> + match_data_pt data;
> + apr_size_t size;
> + } *tls = NULL;
> +
> + /* APR_HAS_THREAD_LOCAL garantees this works */
> + current = apr_thread_current();
> + ap_assert(current != NULL);
> +
> + apr_thread_data_get((void **)&tls, "apreg", current);
> + if (!tls || tls->size < size) {
> + apr_pool_t *tp = apr_thread_pool_get(current);
> + if (tls) {
> +#ifdef HAVE_PCRE2
> + pcre2_match_data_free(tls->data); /* NULL safe */
> +#endif
> + }
> + else {
> + tls = apr_pcalloc(tp, sizeof(*tls));
> + apr_thread_data_set(tls, "apreg", NULL, current);
> + }
> + tls->size *= 2;
> + if (tls->size < size) {
> + tls->size = size;
> + if (tls->size < POSIX_MALLOC_THRESHOLD) {
> + tls->size = POSIX_MALLOC_THRESHOLD;
> + }
> + }
> +#ifdef HAVE_PCRE2
> + tls->data = pcre2_match_data_create(tls->size, NULL);
> +#else
> + tls->data = apr_palloc(tp, tls->size * sizeof(int) * 3);
> +#endif
> + if (!tls->data) {
> + tls->size = 0;
> + return NULL;
> + }
> + }
> +
> +#ifdef HAVE_PCRE2
> + *ovector = pcre2_get_ovector_pointer(tls->data);
> +#else
> + *vector = tls->data;
*ovector instead of *vector?
> +#endif
> + return tls->data;
> +}
> +
> +/* Nothing to put back with thread local */
> +static APR_INLINE void put_match_data(match_data_pt data,
> + apr_size_t size)
> +{ }
> +
> +#else /* !APR_HAS_THREAD_LOCAL */
> +
> +/* Always allocate/free without thread local */
> +
> +static match_data_pt get_match_data(apr_size_t size,
> + match_vector_pt *ovector,
> + match_vector_pt small_vector)
> +{
> + match_data_pt data;
> +
> +#ifdef HAVE_PCRE2
> + data = pcre2_match_data_create(size, NULL);
> + *ovector = pcre2_get_ovector_pointer(data);
Should we do an if (data) here? Not sure if pcre2_get_ovector_pointer can work
with NULL
> +#else
> + if (size > POSIX_MALLOC_THRESHOLD) {
> + data = malloc(size * sizeof(int) * 3);
> + }
> + else {
> + data = small_vector;
> + }
> + *ovector = data;
> +#endif
> +
> + return data;
> +}
> +
> +static APR_INLINE void put_match_data(match_data_pt data,
> + apr_size_t size)
> +{
> +#ifdef HAVE_PCRE2
> + pcre2_match_data_free(data);
> +#else
> + if (size > POSIX_MALLOC_THRESHOLD) {
> + free(data);
> + }
> +#endif
> +}
> +
> +#endif /* !APR_HAS_THREAD_LOCAL */
> +
> AP_DECLARE(int) ap_regexec(const ap_regex_t *preg, const char *string,
> apr_size_t nmatch, ap_regmatch_t *pmatch,
> int eflags)
>
Regards
Rüdiger
