date:20190122

Re: aio/unix: Use signal.sival which is standard

2019-01-22 Thread Maxim Dounin

Hello!

On Thu, Jan 17, 2019 at 06:18:29PM +0300, Sergey Kandaurov wrote:

> 
> > On 17 Jan 2019, at 16:22, Maxim Dounin  wrote:
> > 
> > Hello!
> > 
> > On Thu, Jan 17, 2019 at 01:29:56PM +0300, Sergey Kandaurov wrote:
> > 
> >> 
> >> 
> >>> On 17 Jan 2019, at 08:43, Sepherosa Ziehau  wrote:
> >>> 
> >>> What's the preferred way to handle this?  I am not sure whether you
> >>> guys allow __FreeBSD_version testing etc.
> >>> 
> >> 
> >> This could be solved with autotests.
> > 
> > As long as we only care about different FreeBSD versions, this 
> > might as well be an auto/os/freebsd test based on $version, or 
> > just a define based on __FreeBSD_version in 
> > src/os/unix/ngx_freebsd_config.h.
> > 
> >> # HG changeset patch
> >> # User Sergey Kandaurov 
> >> # Date 1547720890 0
> >> #  Thu Jan 17 10:28:10 2019 +
> >> # Node ID d28513cd71bce227b4e159b7a3f518aa504232f0
> >> # Parent  6d15e452fa2eaf19408e24a0d0fcc3a31344a289
> >> Fixed portability issues with union sigval.
> >> 
> >> The sival_ptr field is now preferably used.
> >> 
> >> diff --git a/auto/unix b/auto/unix
> >> --- a/auto/unix
> >> +++ b/auto/unix
> >> @@ -523,6 +523,7 @@ if [ $NGX_FILE_AIO = YES ]; then
> >> ngx_feature_libs=
> >> ngx_feature_test="struct aiocb  iocb;
> >>   iocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
> >> +  iocb.aio_sigevent.sigev_value.sival_ptr = NULL;
> >>   (void) aio_read(&iocb)"
> >> . auto/feature
> >> 
> >> @@ -532,6 +533,22 @@ if [ $NGX_FILE_AIO = YES ]; then
> >> 
> >> if [ $ngx_found = no ]; then
> >> 
> >> +ngx_feature="kqueue AIO support (legacy)"
> >> +ngx_feature_name="NGX_HAVE_FILE_AIO_LEGACY"
> >> +ngx_feature_test="struct aiocb  iocb;
> >> +  iocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
> >> +  iocb.aio_sigevent.sigev_value.sigval_ptr = NULL;
> >> +  (void) aio_read(&iocb)"
> >> +. auto/feature
> > 
> > Note that this will test for "kqueue AIO support (legacy)" on each 
> > Linux build with aio.
> 
> That is unfortunate.
> 
> >> +
> >> +if [ $ngx_found = yes ]; then
> >> +CORE_SRCS="$CORE_SRCS $FILE_AIO_SRCS"
> >> +have=NGX_HAVE_FILE_AIO . auto/have
> >> +fi
> >> +fi
> >> +
> >> +if [ $ngx_found = no ]; then
> >> +
> >> ngx_feature="Linux AIO support"
> >> ngx_feature_name="NGX_HAVE_FILE_AIO"
> >> ngx_feature_run=no
> >> diff --git a/src/os/unix/ngx_file_aio_read.c 
> >> b/src/os/unix/ngx_file_aio_read.c
> >> --- a/src/os/unix/ngx_file_aio_read.c
> >> +++ b/src/os/unix/ngx_file_aio_read.c
> >> @@ -110,8 +110,12 @@ ngx_file_aio_read(ngx_file_t *file, u_ch
> >> #if (NGX_HAVE_KQUEUE)
> >> aio->aiocb.aio_sigevent.sigev_notify_kqueue = ngx_kqueue;
> >> aio->aiocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
> >> +#if !(NGX_HAVE_FILE_AIO_LEGACY)
> >> +aio->aiocb.aio_sigevent.sigev_value.sival_ptr = ev;
> >> +#else
> >> aio->aiocb.aio_sigevent.sigev_value.sigval_ptr = ev;
> >> #endif
> >> +#endif
> >> ev->handler = ngx_file_aio_event_handler;
> >> 
> >> n = aio_read(&aio->aiocb);
> > 
> > A simplier solution might be to always use sival_ptr, and define 
> > it to sigval_ptr on old FreeBSDs.
> 
> This looks simpler indeed.
> 
> > Alternatively, we can consider dropping file AIO support for these 
> > old FreeBSD versions.  This shouldn't be a big deal as this is an 
> > optional feature which is not enabled by default.
> 
> And that's what I'd prefer for --test-build-eventport workaround.
> 
> What about these patches?
> For DragonFly this should effectively match initial submission.
> 
> # HG changeset patch
> # User Sergey Kandaurov 
> # Date 1547734251 0
> #  Thu Jan 17 14:10:51 2019 +
> # Node ID baab2b35e8cc79cfdf9924d1752348e97c1da13e
> # Parent  6d15e452fa2eaf19408e24a0d0fcc3a31344a289
> Fixed portability issues with union sigval.

This needs to be extended with some background information - 
notably why it used to be sigval_ptr, and why to switch to 
sival_ptr.

> 
> diff --git a/src/os/unix/ngx_file_aio_read.c b/src/os/unix/ngx_file_aio_read.c
> --- a/src/os/unix/ngx_file_aio_read.c
> +++ b/src/os/unix/ngx_file_aio_read.c
> @@ -110,7 +110,7 @@ ngx_file_aio_read(ngx_file_t *file, u_ch
>  #if (NGX_HAVE_KQUEUE)
>  aio->aiocb.aio_sigevent.sigev_notify_kqueue = ngx_kqueue;
>  aio->aiocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
> -aio->aiocb.aio_sigevent.sigev_value.sigval_ptr = ev;
> +aio->aiocb.aio_sigevent.sigev_value.sival_ptr = ev;
>  #endif
>  ev->handler = ngx_file_aio_event_handler;
>  
> diff --git a/src/os/unix/ngx_freebsd_config.h 
> b/src/os/unix/ngx_freebsd_config.h
> --- a/src/os/unix/ngx_freebsd_config.h
> +++ b/src/os/unix/ngx_freebsd_config.h
> @@ -91,6 +91,10 @@
>  #if (NGX_HAVE_FILE_AIO)
>  #include 
>  typedef struct aiocb  ngx_aiocb_t;
> +
> +#if (__FreeBSD_version < 75 && !

[njs] Improved setting vm->trace.

2019-01-22 Thread Dmitry Volyntsev

details:   https://hg.nginx.org/njs/rev/11cfd1d486f7
branches:  
changeset: 735:11cfd1d486f7
user:  Dmitry Volyntsev 
date:  Tue Jan 22 18:08:47 2019 +0300
description:
Improved setting vm->trace.

diffstat:

 njs/njs.c |  6 +-
 1 files changed, 1 insertions(+), 5 deletions(-)

diffs (23 lines):

diff -r 0813395557b6 -r 11cfd1d486f7 njs/njs.c
--- a/njs/njs.c Fri Jan 18 15:28:17 2019 +0300
+++ b/njs/njs.c Tue Jan 22 18:08:47 2019 +0300
@@ -335,6 +335,7 @@ njs_vm_clone(njs_vm_t *vm, njs_external_
 nvm->mem_cache_pool = nmcp;
 
 nvm->shared = vm->shared;
+nvm->trace = vm->trace;
 
 nvm->variables_hash = vm->variables_hash;
 nvm->values_hash = vm->values_hash;
@@ -444,11 +445,6 @@ njs_vm_init(njs_vm_t *vm)
 vm->backtrace = backtrace;
 }
 
-vm->trace.level = NXT_LEVEL_TRACE;
-vm->trace.size = 2048;
-vm->trace.handler = njs_parser_trace_handler;
-vm->trace.data = vm;
-
 if (njs_is_null(&vm->retval)) {
 vm->retval = njs_value_void;
 }
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: cache: move open to thread pool

2019-01-22 Thread Ka-Hing Cheung via nginx-devel

Hi Maxim and Roman,

We've been running a version of your patches in our test colo for the
past week with no ill effects, with the caveat that it's difficult to
measure performance impacts in our test colo because of varying
traffic. We had to massage the patches a bit because we don't run
vanilla nginx. Also we've only tried with open file cache disabled.

Let me know if you are looking for other feedback.

- Ka-Hing

On Mon, Jan 14, 2019 at 5:53 PM Ka-Hing Cheung  wrote:
>
> We didn't get to it last year because of other priorities but this is
> being worked on. We don't use a vanilla nginx so it takes effort to
> integrate your patches. Having a version of this in upstream nginx is
> important to us and to me personally.
>
> On Thu, Jan 10, 2019 at 12:30 AM Maxim Konovalov  wrote:
> >
> > So guys, are you really interested in this work beyond shiny
> > marketing blog posts?
> >
> > On 05/12/2018 12:43, Maxim Konovalov wrote:
> > > Hello,
> > >
> > > just a reminder that we are looking for a tester for these patches.
> > >
> > > Thanks,
> > >
> > > Maxim
> > >
> > > On 16/11/2018 12:10, Maxim Konovalov wrote:
> > >> Thanks, Ka-Hing, we'll wait for your feedback.
> > >>
> > >> On 16/11/2018 01:53, Ka-Hing Cheung wrote:
> > >>> Hi,
> > >>>
> > >>> I didn't forget about this. I am pretty swamped at the moment and
> > >>> there's a holiday freeze coming up. Will get to his in December.
> > >>>
> > >>> - Ka-Hing
> > >>>
> > >>> On Thu, Nov 8, 2018 at 6:19 AM Maxim Konovalov  wrote:
> > 
> >  Hi Ka-Hing,
> > 
> >  would you mind to test Roman's most recent patches that add
> >  "aio_open" directive?
> > 
> >  http://mailman.nginx.org/pipermail/nginx-devel/2018-November/011538.html
> > 
> >  We are looking for overall performance and stability metrics and
> >  feedback.
> > 
> >  Much appreciated,
> > 
> >  Maxim
> > 
> >  --
> >  Maxim Konovalov
> > >>
> > >>
> > >
> > >
> >
> >
> > --
> > Maxim Konovalov
> > ___
> > nginx-devel mailing list
> > nginx-devel@nginx.org
> > http://mailman.nginx.org/mailman/listinfo/nginx-devel
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: cache: move open to thread pool

2019-01-22 Thread Ka-Hing Cheung via nginx-devel

I spoke too soon, just realized our test colo is running the patches
without aio_open on. Flipping that switch now.

Also, including the patch that we applied on top in addition to the
massaging we did to resolve conflicts. I haven't dug too deep to see
if stock nginx also requires similar changes or they are only
necessary because of our other nginx changes:

--- src/http/ngx_http_file_cache.c
+++ src/http/ngx_http_file_cache.c
@@ -611,6 +611,12 @@ ngx_http_do_file_cache_open(ngx_http_request_t
*r, unsigned ignore_vary)
 return NGX_ERROR;
 }

+c->buf = ngx_create_temp_buf(r->pool, c->body_start);
+if (c->buf == NULL) {
+return NGX_ERROR;
+}
+c->file.fd = NGX_INVALID_FILE;
+
 if (!test) {
 /*
  * Either the request is cacheable but the file doesn't
@@ -1148,6 +1154,7 @@ ngx_http_file_cache_aio_open(ngx_http_request_t
*r, ngx_http_cache_t *c)
 ngx_memzero(&r->open_file_info, sizeof(ngx_open_file_info_t));

 r->open_file_info.uniq = c->uniq;
+r->open_file_info.fd = NGX_INVALID_FILE;
 r->open_file_info.valid = clcf->open_file_cache_valid;
 r->open_file_info.min_uses = clcf->open_file_cache_min_uses;
 r->open_file_info.events = clcf->open_file_cache_events;
@@ -1205,11 +1212,6 @@ ngx_http_file_cache_aio_open(ngx_http_request_t
*r, ngx_http_cache_t *c)
 c->length = r->open_file_info.size;
 c->fs_size = (r->open_file_info.fs_size + cache->bsize - 1) / cache->bsize;

-c->buf = ngx_create_temp_buf(r->pool, c->body_start);
-if (c->buf == NULL) {
-return NGX_ERROR;
-}
-
 return NGX_OK;
 }

On Tue, Jan 22, 2019 at 10:20 AM Ka-Hing Cheung  wrote:
>
> Hi Maxim and Roman,
>
> We've been running a version of your patches in our test colo for the
> past week with no ill effects, with the caveat that it's difficult to
> measure performance impacts in our test colo because of varying
> traffic. We had to massage the patches a bit because we don't run
> vanilla nginx. Also we've only tried with open file cache disabled.
>
> Let me know if you are looking for other feedback.
>
> - Ka-Hing
>
> On Mon, Jan 14, 2019 at 5:53 PM Ka-Hing Cheung  wrote:
> >
> > We didn't get to it last year because of other priorities but this is
> > being worked on. We don't use a vanilla nginx so it takes effort to
> > integrate your patches. Having a version of this in upstream nginx is
> > important to us and to me personally.
> >
> > On Thu, Jan 10, 2019 at 12:30 AM Maxim Konovalov  wrote:
> > >
> > > So guys, are you really interested in this work beyond shiny
> > > marketing blog posts?
> > >
> > > On 05/12/2018 12:43, Maxim Konovalov wrote:
> > > > Hello,
> > > >
> > > > just a reminder that we are looking for a tester for these patches.
> > > >
> > > > Thanks,
> > > >
> > > > Maxim
> > > >
> > > > On 16/11/2018 12:10, Maxim Konovalov wrote:
> > > >> Thanks, Ka-Hing, we'll wait for your feedback.
> > > >>
> > > >> On 16/11/2018 01:53, Ka-Hing Cheung wrote:
> > > >>> Hi,
> > > >>>
> > > >>> I didn't forget about this. I am pretty swamped at the moment and
> > > >>> there's a holiday freeze coming up. Will get to his in December.
> > > >>>
> > > >>> - Ka-Hing
> > > >>>
> > > >>> On Thu, Nov 8, 2018 at 6:19 AM Maxim Konovalov  
> > > >>> wrote:
> > > 
> > >  Hi Ka-Hing,
> > > 
> > >  would you mind to test Roman's most recent patches that add
> > >  "aio_open" directive?
> > > 
> > >  http://mailman.nginx.org/pipermail/nginx-devel/2018-November/011538.html
> > > 
> > >  We are looking for overall performance and stability metrics and
> > >  feedback.
> > > 
> > >  Much appreciated,
> > > 
> > >  Maxim
> > > 
> > >  --
> > >  Maxim Konovalov
> > > >>
> > > >>
> > > >
> > > >
> > >
> > >
> > > --
> > > Maxim Konovalov
> > > ___
> > > nginx-devel mailing list
> > > nginx-devel@nginx.org
> > > http://mailman.nginx.org/mailman/listinfo/nginx-devel
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: aio/unix: Use signal.sival which is standard

[njs] Improved setting vm->trace.

Re: cache: move open to thread pool

Re: cache: move open to thread pool

4 matches

Site Navigation

Mail list logo

Footer information