Disregard * 2 I understand now. On Thu, 16 Apr 2020 at 13:14, Mathew Heard <m...@mheard.com> wrote:
> Disregard the previous email, there was a typo there. > > Maxim, > > I'm doing line by line documentation in my module. I hope by doing this I > will get a good understanding of what is going on. If allowed I intend to > commit this as a resource for others also intending to learn the body > filter system (I will however check with this mailing list to see if anyone > has an issue with my descriptions). > > While doing this I noticed that ctx->busy does not appear to ever be true > in your gunzip module. Am I missing something here? > > Regards, > Mathew > > > On Thu, 16 Apr 2020 at 13:11, Mathew Heard <m...@mheard.com> wrote: > >> Maxim, >> >> I'm doing line by line documentation in my module. I hope by doing this I >> will get a good understanding of what is going on. If allowed I intend to >> commit this as a resource for others also intending to learn the body >> filter system (I will however check with this mailing list to see if anyone >> has an issue with my descriptions). >> >> While doing this I noticed that ctx->flush does not appear to ever be >> true in your gunzip module. Am I missing something here? >> >> Regards, >> Mathew >> >> On Thu, 16 Apr 2020 at 10:49, Mathew Heard <m...@mheard.com> wrote: >> >>> Maxim, >>> >>> Which clause of the 2 clause BSD license am I violating? It's not my >>> intention to violate any. If need be I will remove this project from >>> distribution and take it closed source. It would be a shame but if it needs >>> to be done... >>> >>> >> Redistribution and use in source and binary forms, with or without >>> modification, are permitted provided that the following conditions are met: >>> >>> Agreed, distribution with modification. >>> >>> >> Redistributions of source code must retain the above copyright >>> notice, this list of conditions and the following disclaimer. >>> >>> https://github.com/splitice/ngx_brunzip_module/blob/master/LICENSE.md >>> >>> >> Redistributions in binary form must reproduce the above copyright >>> >> notice, >>> this list of conditions and the following disclaimer in the documentation >>> and/or other materials provided with the distribution. >>> >>> No binary distribution is performed. >>> >>> >> [etcetera] >>> >>> There is no express intent to apply any warranty to you or Nginx inc. >>> >>> I'll add some copyright lines at the top of that file as those should >>> probably be there, I'm not sure they have any legal implication (copyright >>> is inheint) but better to give credit for the functions used as a reference. >>> >>> As for the assistance I am digesting that now. I see what you mean >>> though regarding available_out == 0, that's indeed a problem. I'll go >>> through the path for my < 64 patch too, that was not the intent. >>> >>> I'm aware of that documentation, I guess I was hoping that there was >>> more. >>> >>> Regards, >>> Mathew >>> >>> On Thu, 16 Apr 2020 at 03:12, Maxim Dounin <mdou...@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@nginx.org >>>> http://mailman.nginx.org/mailman/listinfo/nginx >>>> >>>
_______________________________________________ nginx mailing list nginx@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx
