Nginx Brunzip

[Mathew Heard]

Maxim,

> Likely unrealted, but "ctx->flush = FLUSH_NOFLUSH" at
>
https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L393
> is meaningless.

Is beause of
https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L513
correct?
Because FLUSH_FLUSH always resets state to FLUSH_NOFLUSH.

I've been working on a heavily commented module. I welcome any feedback on
the comments (or the areas marked with "???" that I am still trying to
figure out).

I'll continue working to figure out the filter system.

Regards,
Mathew

On Thu, 16 Apr 2020 at 03:12, Maxim Dounin <mdounin at mdounin.ru> wrote:

> Hello!
>
> On Wed, Apr 15, 2020 at 10:21:09AM +1000, Mathew Heard wrote:
>
> > Hi all,
> >
> > I'm the maintainer of an open source module ngx_brunzip_module (
> > https://github.com/splitice/ngx_brunzip_module/
> > <
> https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c
> >).
> > Effectively the same as the gunzip module (and based off that source) but
> > with Brotli.
>
> If it's based on the gunzip module code, you are violating
> copyrights on the code, including mine.  Please fix.
>
> > I've been scratching my head for 2 days regarding some high CPU usage
> > within the chain code. It appears that some spinning is possible. I must
> > admit I only have a basic understanding of the filter chain in nginx
> (still
> > gaining experience).
> >
> > 1. I was wondering if someone could take a look at the code and give me
> > some pointers?
>
> Likely unrelated, but "ctx->input" and "ctx->output" are
> meaningless and never used.
>
> Likely unrealted, but "ctx->flush = FLUSH_NOFLUSH" at
>
> https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L393
> is meaningless.
>
> > 2. Also I've added some code to prevent further filling of mostly full
> > buffers (as it appears brotli is quite expensive to start) at
> >
> https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L408
> > is
> > this valid? How does nginx determine when backpressure from full output
> > chains is relieved? Is there any in-depth documentation of the filter
> chain
> > architecture?
>
> No, it's not valid, and your code will throw away such mostly
> filled output buffers without linking them to the output chain as
> normally happens in ngx_http_brunzip_filter_inflate() at
>
>
> https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L485
>
> Further, the test in question looks incorrect, as it doesn't
> take into account the edge case when amount of the output returned
> by BrotliDecoderDecompressStream() exactly matches the output
> buffer size, so ctx->available_out is 0, but rc is not
> BROTLI_DECODER_RESULT_NEEDS_MORE_OUTPUT.
>
> As for the documentation, it looks like you are looking for the
> documentation of the code in the module.  You may want to re-read
> it to understand (and fix the copyright as requested above to
> admit that you aren't the one who wrote most of the code).  Some
> basics about buffers and chains can be found here:
>
> http://nginx.org/en/docs/dev/development_guide.html#buffer
>
> Some simplified example of a code to work with buffers reuse as
> used by the module can be found here:
>
> http://nginx.org/en/docs/dev/development_guide.html#http_body_buffers_reuse
>
> Other chapters, such as "Code style", might be helpful as well.
> Also, don't hesitate to look into the code of the functions you
> are using, it often helps.
>
> --
> Maxim Dounin
> http://mdounin.ru/
> _______________________________________________
> nginx mailing list
> nginx at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx/attachments/20200416/f905bc68/attachment.htm>