You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rj...@apache.org on 2015/02/05 21:33:59 UTC
svn commit: r1657685 - /httpd/httpd/trunk/server/util_expr_eval.c
Author: rjung
Date: Thu Feb 5 20:33:59 2015
New Revision: 1657685
URL: http://svn.apache.org/r1657685
Log:
Expression parser: Optimize another concatenation
case by using iteration instead of recursion.
We have a relatively small recursion limit of
about 10 operations. This is a compilation
limit (a define). It can be hit if many expr
vars or function calls are concatenated.
The new optimization is very similar to the
existing one, which optimizes consecutive
concatenations in node2 of the tree. The new
one optimizes consecutive concatenations in
node 1.
Modified:
httpd/httpd/trunk/server/util_expr_eval.c
Modified: httpd/httpd/trunk/server/util_expr_eval.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_expr_eval.c?rev=1657685&r1=1657684&r2=1657685&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util_expr_eval.c (original)
+++ httpd/httpd/trunk/server/util_expr_eval.c Thu Feb 5 20:33:59 2015
@@ -103,7 +103,8 @@ static const char *ap_expr_eval_word(ap_
node->node_arg2);
break;
case op_Concat:
- if (((ap_expr_t *)node->node_arg2)->node_op != op_Concat) {
+ if (((ap_expr_t *)node->node_arg2)->node_op != op_Concat &&
+ ((ap_expr_t *)node->node_arg1)->node_op != op_Concat) {
const char *s1 = ap_expr_eval_word(ctx, node->node_arg1);
const char *s2 = ap_expr_eval_word(ctx, node->node_arg2);
if (!*s1)
@@ -113,6 +114,30 @@ static const char *ap_expr_eval_word(ap_
else
result = apr_pstrcat(ctx->p, s1, s2, NULL);
}
+ else if (((ap_expr_t *)node->node_arg1)->node_op == op_Concat) {
+ const ap_expr_t *nodep = node;
+ int n;
+ int i = 1;
+ struct iovec *vec;
+ do {
+ nodep = nodep->node_arg1;
+ i++;
+ } while (nodep->node_op == op_Concat);
+ vec = apr_palloc(ctx->p, i * sizeof(struct iovec));
+ n = i;
+ nodep = node;
+ i--;
+ do {
+ vec[i].iov_base = (void *)ap_expr_eval_word(ctx,
+ nodep->node_arg2);
+ vec[i].iov_len = strlen(vec[i].iov_base);
+ i--;
+ nodep = nodep->node_arg1;
+ } while (nodep->node_op == op_Concat);
+ vec[i].iov_base = (void *)ap_expr_eval_word(ctx, nodep);
+ vec[i].iov_len = strlen(vec[i].iov_base);
+ result = apr_pstrcatv(ctx->p, vec, n, NULL);
+ }
else {
const ap_expr_t *nodep = node;
int i = 1;
Re: svn commit: r1657685 - /httpd/httpd/trunk/server/util_expr_eval.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Feb 24, 2015 at 2:12 PM, Rainer Jung <ra...@kippdata.de> wrote:
>
> Am 24.02.2015 um 12:51 schrieb Yann Ylavic:
>>
>> Why concatenating in reverse order here?
>
> Because first doing it in normal order produced a reverse result and my
> paper computer then showed reverse is correct :)
That's why I wanted to test before asking too :)
>
> Assume the tree looks like (left is node1, right node2):
>
> concat
> concat r1
> concat r2
> l1 r3
>
> Result should be l1r3r2r1, so the r's need reverse order.
Makes sense now (gdb helped me too...).
>>
>> Can you please share an expression for the new branch to be taken?
>
>
> The problem (recursion limit reached because the previous optimization
> didn't apply) occurred for me with string expressions (not boolean) of the
> form
>
> "constant1%{function1:arg1}constant2%{function2:arg2}constant3%{function3:arg3}..."
I wanted to use this kind of expression, without the "." operator and
its right association, but that's not allowed with boolean
expressions.
The thing was *string* expressions, which I didn't think about...
Thanks for the explanation and the fix!
It's working as expected, +1.
Regards,
Yann.
Re: svn commit: r1657685 - /httpd/httpd/trunk/server/util_expr_eval.c
Posted by Rainer Jung <ra...@kippdata.de>.
Hi Yann,
Am 24.02.2015 um 12:51 schrieb Yann Ylavic:
> Hi,
>
> On Thu, Feb 5, 2015 at 9:33 PM, <rj...@apache.org> wrote:
>> Author: rjung
>> Date: Thu Feb 5 20:33:59 2015
>> New Revision: 1657685
>>
>> URL: http://svn.apache.org/r1657685
> [...]
>>
>> Modified:
>> httpd/httpd/trunk/server/util_expr_eval.c
>>
>> Modified: httpd/httpd/trunk/server/util_expr_eval.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_expr_eval.c?rev=1657685&r1=1657684&r2=1657685&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/server/util_expr_eval.c (original)
>> +++ httpd/httpd/trunk/server/util_expr_eval.c Thu Feb 5 20:33:59 2015
> [...]
>> @@ -113,6 +114,30 @@ static const char *ap_expr_eval_word(ap_
>> else
>> result = apr_pstrcat(ctx->p, s1, s2, NULL);
>> }
>> + else if (((ap_expr_t *)node->node_arg1)->node_op == op_Concat) {
>> + const ap_expr_t *nodep = node;
>> + int n;
>> + int i = 1;
>> + struct iovec *vec;
>> + do {
>> + nodep = nodep->node_arg1;
>> + i++;
>> + } while (nodep->node_op == op_Concat);
>> + vec = apr_palloc(ctx->p, i * sizeof(struct iovec));
>> + n = i;
>> + nodep = node;
>> + i--;
>> + do {
>> + vec[i].iov_base = (void *)ap_expr_eval_word(ctx,
>> + nodep->node_arg2);
>> + vec[i].iov_len = strlen(vec[i].iov_base);
>> + i--;
>> + nodep = nodep->node_arg1;
>> + } while (nodep->node_op == op_Concat);
>> + vec[i].iov_base = (void *)ap_expr_eval_word(ctx, nodep);
>> + vec[i].iov_len = strlen(vec[i].iov_base);
>> + result = apr_pstrcatv(ctx->p, vec, n, NULL);
>> + }
>> else {
>
> Why concatenating in reverse order here?
Because first doing it in normal order produced a reverse result and my
paper computer then showed reverse is correct :)
Assume the tree looks like (left is node1, right node2):
concat
concat r1
concat r2
l1 r3
Result should be l1r3r2r1, so the r's need reverse order.
> Actually, I cannot manage to test this because I'm not able to find an
> expression that produces node_arg1->node_op == op_Concat.
> Due to the %right association of T_OP_CONCAT in the parser/yacc, "'x'
> . 'y' . 'z'" gives "concat( 'x', concat ('y, 'z'))", which is
> node_arg1->node_op == op_String and node_arg2->node_op == op_Concat...
>
> Can you please share an expression for the new branch to be taken?
The problem (recursion limit reached because the previous optimization
didn't apply) occurred for me with string expressions (not boolean) of
the form
"constant1%{function1:arg1}constant2%{function2:arg2}constant3%{function3:arg3}..."
For example in 2.4 use mod_log_debug and the following config:
LogLevel info
<Location />
LogMessage
"|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}|%{HTTPS}"
</Location>
Without the patch the error log contains:
[Tue Feb 24 14:06:48.913709 2015] [log_debug:error] [pid 3664:tid 22]
[client 127.0.0.1:48807] Evaluation of expression from
/.../conf/httpd.conf:563 failed: Recursion limit reached
[Tue Feb 24 14:06:48.913805 2015] [log_debug:error] [pid 3664:tid 22]
[client 127.0.0.1:48807] AH00641: Can't evaluate message expression:
Recursion limit reached
[Tue Feb 24 14:06:48.913827 2015] [log_debug:info] [pid 3664:tid 22]
[client 127.0.0.1:48807] (null)
With the patch you get:
[Tue Feb 24 14:10:50.497472 2015] [log_debug:info] [pid 8785:tid 27]
[client 127.0.0.1:48817]
|off|off|off|off|off|off|off|off|off|off|off|off|off|off|off
Thank you for your careful review!
Regards,
Rainer
Re: svn commit: r1657685 - /httpd/httpd/trunk/server/util_expr_eval.c
Posted by Yann Ylavic <yl...@gmail.com>.
Hi,
On Thu, Feb 5, 2015 at 9:33 PM, <rj...@apache.org> wrote:
> Author: rjung
> Date: Thu Feb 5 20:33:59 2015
> New Revision: 1657685
>
> URL: http://svn.apache.org/r1657685
[...]
>
> Modified:
> httpd/httpd/trunk/server/util_expr_eval.c
>
> Modified: httpd/httpd/trunk/server/util_expr_eval.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_expr_eval.c?rev=1657685&r1=1657684&r2=1657685&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_expr_eval.c (original)
> +++ httpd/httpd/trunk/server/util_expr_eval.c Thu Feb 5 20:33:59 2015
[...]
> @@ -113,6 +114,30 @@ static const char *ap_expr_eval_word(ap_
> else
> result = apr_pstrcat(ctx->p, s1, s2, NULL);
> }
> + else if (((ap_expr_t *)node->node_arg1)->node_op == op_Concat) {
> + const ap_expr_t *nodep = node;
> + int n;
> + int i = 1;
> + struct iovec *vec;
> + do {
> + nodep = nodep->node_arg1;
> + i++;
> + } while (nodep->node_op == op_Concat);
> + vec = apr_palloc(ctx->p, i * sizeof(struct iovec));
> + n = i;
> + nodep = node;
> + i--;
> + do {
> + vec[i].iov_base = (void *)ap_expr_eval_word(ctx,
> + nodep->node_arg2);
> + vec[i].iov_len = strlen(vec[i].iov_base);
> + i--;
> + nodep = nodep->node_arg1;
> + } while (nodep->node_op == op_Concat);
> + vec[i].iov_base = (void *)ap_expr_eval_word(ctx, nodep);
> + vec[i].iov_len = strlen(vec[i].iov_base);
> + result = apr_pstrcatv(ctx->p, vec, n, NULL);
> + }
> else {
Why concatenating in reverse order here?
Actually, I cannot manage to test this because I'm not able to find an
expression that produces node_arg1->node_op == op_Concat.
Due to the %right association of T_OP_CONCAT in the parser/yacc, "'x'
. 'y' . 'z'" gives "concat( 'x', concat ('y, 'z'))", which is
node_arg1->node_op == op_String and node_arg2->node_op == op_Concat...
Can you please share an expression for the new branch to be taken?
Regards,
Yann.