[PATCH 11 of 14] Proxy: split configured header names and values

[Maxim Dounin]

Hello!

On Thu, Jun 22, 2017 at 01:33:15PM -0700, Piotr Sikora via nginx-devel wrote:

> # HG changeset patch
> # User Piotr Sikora <piotrsikora at google.com>
> # Date 1489618535 25200
> #      Wed Mar 15 15:55:35 2017 -0700
> # Node ID 0637acdb51e29e1f68f5f3e762115c702cab4e72
> # Parent  068381014f256ad6e2dc490bacc2529cebbb0462
> Proxy: split configured header names and values.
> 
> Previously, each configured header was represented in one of two ways,
> depending on whether or not its value included any variables.
> 
> If the value didn't include any variables, then it would be represented
> as as a single script that contained complete header line with HTTP/1.1
> delimiters, i.e.:
> 
>      "Header: value\r\n"
> 
> But if the value included any variables, then it would be represented
> as a series of three scripts: first contained header name and the ": "
> delimiter, second evaluated to header value, and third contained only
> "\r\n", i.e.:
> 
>      "Header: "
>      "$value"
>      "\r\n"
> 
> This commit changes that, so that each configured header is represented
> as a series of two scripts: first contains only header name, and second
> contains (or evaluates to) only header value, i.e.:
> 
>     "Header"
>     "$value"
> 
> or
> 
>     "Header"
>     "value"
> 
> This not only makes things more consistent, but also allows header name
> and value to be accessed separately.
> 
> Signed-off-by: Piotr Sikora <piotrsikora at google.com>

As suggested during previous review iteration, it would be 
interesting to see some benchmarks.  Here they are.

With 1000 static headers "proxy_set_header X-Foo foo;" the new 
variant is slightly slower (numbers are in requests per second, 
higher is better):

$ ministat t.current.n t.patched.n
x t.current.n
+ t.patched.n
+------------------------------------------------------------------------------+
|+        +           +   +  ++ +    +                  x   x  x       x xx   x|
|          |___________A_____M_____|                        |_______A__M____|  |
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   7       793.883       820.497       811.498     807.94229     10.111563
+   8       725.285       770.099        759.39     752.68988     15.026166
Difference at 95.0% confidence
	-55.2524 +/- 14.5227
	-6.83866% +/- 1.7975%
	(Student's t, pooled s = 12.991)

It is considerably faster when using variables though, again with 
1000 headers "proxy_set_header X-Foo $request_uri;":

$ ministat t.current.vars.n t.patched.vars.n 
x t.current.vars.n
+ t.patched.vars.n
+------------------------------------------------------------------------------+
|     x                                                                  ++ +  |
|x  xxx     x                                           +             +  ++ +  |
| |___A__|                                                       |______A_M___||
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   8       365.499        415.19       385.856       386.026     14.078621
+   8       605.499       690.492       681.396     671.80087     27.852872
Difference at 95.0% confidence
	285.775 +/- 23.6679
	74.03% +/- 6.13117%
	(Student's t, pooled s = 22.068)

Given that using variables is much more common, overral the patch 
looks clearly beneficial.

> diff -r 068381014f25 -r 0637acdb51e2 src/http/modules/ngx_http_proxy_module.c
> --- a/src/http/modules/ngx_http_proxy_module.c
> +++ b/src/http/modules/ngx_http_proxy_module.c
> @@ -1151,6 +1151,7 @@ static ngx_int_t
>  ngx_http_proxy_create_request(ngx_http_request_t *r)
>  {
>      size_t                        len, uri_len, loc_len, body_len;
> +    size_t                        key_len, val_len;

Style, should be only one type instead:

-    size_t                        len, uri_len, loc_len, body_len;
-    size_t                        key_len, val_len;
+    size_t                        len, uri_len, loc_len, body_len,
+                                  key_len, val_len;

[...]

Committed with the above change, thanks.

-- 
Maxim Dounin
http://nginx.org/