Re: [PATCH] Core: slight optimization in ngx_chain_update_chains()

[Maxim Dounin]

Hello!

On Thu, Nov 10, 2016 at 10:30:34AM +0800, 胡聪 (hucc) wrote:

> Hi,
> 
> I found that sometimes the *out is NULL when i was reading the code
> of ngx_event_pipe.c .  So it is not necessary to traverse *busy when
> *out == NULL in ngx_chain_update_chains().
> 
> # HG changeset patch
> # User hucongcong <hucon...@foxmail.com>
> # Date 1478744273 -28800
> #      Thu Nov 10 10:17:53 2016 +0800
> # Node ID c7b6269faec0d2ee6b4f4f625db8c219b5d0b010
> # Parent  92ad1c92bcf93310bf59447dd581cac37af87adb
> Core: slight optimization in ngx_chain_update_chains().
> 
> It is not necessary to traverse *busy and link the *out
> when *out == NULL.
> 
> diff -r 92ad1c92bcf9 -r c7b6269faec0 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      Thu Nov 10 10:17:53 2016 +0800
> @@ -189,7 +189,7 @@ ngx_chain_update_chains(ngx_pool_t *p, n
>      if (*busy == NULL) {
>          *busy = *out;
>  
> -    } else {
> +    } else if (*out) {
>          for (cl = *busy; cl->next; cl = cl->next) { /* void */ }
>  
>          cl->next = *out;

Looking into this again I tend to think that better solution would 
be to test *out in additional if around all operations with *out, 
like this (diff -w for clarity):

@@ -186,6 +186,7 @@ ngx_chain_update_chains(ngx_pool_t *p, n
 {
     ngx_chain_t  *cl;
 
+    if (*out) {
     if (*busy == NULL) {
         *busy = *out;
 
@@ -196,6 +197,7 @@ ngx_chain_update_chains(ngx_pool_t *p, n
     }
 
     *out = NULL;
+    }
 
     while (*busy) {
         cl = *busy;

Your patch updated to do this:

# HG changeset patch
# User hucongcong <hucon...@foxmail.com>
# Date 1478744273 -28800
#      Thu Nov 10 10:17:53 2016 +0800
# Node ID 40c2f3e06d2378dea729eae9ddc381928853238d
# Parent  92ad1c92bcf93310bf59447dd581cac37af87adb
Core: slight optimization in ngx_chain_update_chains().

It is not necessary to traverse *busy and link the *out when *out is NULL.

diff --git a/src/core/ngx_buf.c b/src/core/ngx_buf.c
--- a/src/core/ngx_buf.c
+++ b/src/core/ngx_buf.c
@@ -186,17 +186,19 @@ ngx_chain_update_chains(ngx_pool_t *p, n
 {
     ngx_chain_t  *cl;
 
-    if (*busy == NULL) {
-        *busy = *out;
+    if (*out) {
+        if (*busy == NULL) {
+            *busy = *out;
 
-    } else {
-        for (cl = *busy; cl->next; cl = cl->next) { /* void */ }
+        } else {
+            for (cl = *busy; cl->next; cl = cl->next) { /* void */ }
 
-        cl->next = *out;
+            cl->next = *out;
+        }
+
+        *out = NULL;
     }
 
-    *out = NULL;
-
     while (*busy) {
         cl = *busy;
 

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

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel