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@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx