Maybe a little optimization - Yes?

Maxim Dounin mdounin at mdounin.ru
Wed Nov 9 18:07:30 UTC 2016


Hello!

On Tue, Nov 08, 2016 at 10:56:56PM +0800, 胡聪 (hucc) wrote:

> Hi,
> 
> Maybe a little optimization. I am not sure whether nginx intends to do it.
> 
> And this his is the result of nginx-tests:
> All tests successful.
> Files=270, Tests=3328, 259 wallclock secs ( 1.35 usr  0.69 sys + 39.76 cusr  9.54 csys = 51.34 CPU)
> Result: PASS
> 
> # HG changeset patch
> # User hucongcong <hucong.c at foxmail.com>
> # Date 1478615841 -28800
> #      Tue Nov 08 22:37:21 2016 +0800
> # Node ID 4eb871095d2703fbc4ef55edd918bfd9a4011610
> # Parent  92ad1c92bcf93310bf59447dd581cac37af87adb
> Optimize some code.
> 
> *) It is not necessary to traverse *busy and link the *out when *out == NULL.
> *) Only call ngx_array_init() in main request to avoid extra initialization in subrequest.

A bulleted list in the patch description is a good indicator that 
there should be more than one patch.

> 
> diff -r 92ad1c92bcf9 -r 4eb871095d27 src/core/ngx_buf.c
> --- a/src/core/ngx_buf.c	Fri Nov 04 19:12:19 2016 +0300
> +++ b/src/core/ngx_buf.c	Tue Nov 08 22:37:21 2016 +0800
> @@ -189,7 +189,7 @@
>      if (*busy == NULL) {
>          *busy = *out;
>  
> -    } else {
> +    } else if (*out != NULL) {

Style: it should be (*out), the "!= NULL" part is not needed.  
Compare to the "cl->next" test below:

>          for (cl = *busy; cl->next; cl = cl->next) { /* void */ }
>  
>          cl->next = *out;
> diff -r 92ad1c92bcf9 -r 4eb871095d27 src/http/modules/ngx_http_range_filter_module.c
> --- a/src/http/modules/ngx_http_range_filter_module.c	Fri Nov 04 19:12:19 2016 +0300
> +++ b/src/http/modules/ngx_http_range_filter_module.c	Tue Nov 08 22:37:21 2016 +0800
> @@ -224,12 +224,6 @@
>  
>      ctx->offset = r->headers_out.content_offset;
>  
> -    if (ngx_array_init(&ctx->ranges, r->pool, 1, sizeof(ngx_http_range_t))
> -        != NGX_OK)
> -    {
> -        return NGX_ERROR;
> -    }
> -
>      ranges = r->single_range ? 1 : clcf->max_ranges;
>  
>      switch (ngx_http_range_parse(r, ctx, ranges)) {
> @@ -291,6 +285,12 @@
>          }
>      }
>  
> +    if (ngx_array_init(&ctx->ranges, r->pool, 1, sizeof(ngx_http_range_t))
> +        != NGX_OK)
> +    {
> +        return NGX_ERROR;
> +    }
> +
>      p = r->headers_in.range->value.data + 6;
>      size = 0;
>      content_length = r->headers_out.content_length_n;

Please use

[diff]
showfunc=1

in your Mercurial configuration (~/.hgrc) to simplify reading 
patches.  Thanks.

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



More information about the nginx-devel mailing list