[PATCH] ngx_event_pipe: remove unnecessary timer

Maxim Dounin mdounin at mdounin.ru
Mon Oct 19 17:38:34 UTC 2015


Hello!

On Sat, Oct 17, 2015 at 11:43:29PM +0800, quink wrote:

> A timer of 1 msec is not necessary take into account the resolutions of
> ngx_time() and p->start.
> If we don't consider the truncation error of ngx_time() and p->start, '1'
> should be '1000'. If we
> consider the truncation error, don't add timer if limit == 0 and discard '+
> 1'.
> 
> ---
>  src/event/ngx_event_pipe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/event/ngx_event_pipe.c b/src/event/ngx_event_pipe.c
> index 2d0e7d3..580a947 100644
> --- a/src/event/ngx_event_pipe.c
> +++ b/src/event/ngx_event_pipe.c
> @@ -181,9 +181,9 @@ ngx_event_pipe_read_upstream(ngx_event_pipe_t *p)
>                  limit = (off_t) p->limit_rate * (ngx_time() - p->start_sec
> + 1)
>                          - p->read_length;
> 
> -                if (limit <= 0) {
> +                if (limit < 0) {
>                      p->upstream->read->delayed = 1;
> -                    delay = (ngx_msec_t) (- limit * 1000 / p->limit_rate +
> 1);
> +                    delay = (ngx_msec_t) (- limit * 1000 / p->limit_rate);
>                      ngx_add_timer(p->upstream->read, delay);
>                      break;
>                  }

If we've already exhausted allowed bandwidth (limit == 0), no 
further oprations will be possible.  So it doesn't make sense to 
try to read further data in such case.

Additionally, with the proposed patch things become just wrong, 
because as a result of the code limit is set to 0, which is later 
interpreted as "no limit is set" by the recv_chain() handler.

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



More information about the nginx-devel mailing list