Hello!

On Sat, Oct 07, 2023 at 06:09:18AM +0200, Davood Falahati wrote:

> # HG changeset patch
> # User Davood Falahati <0x0davood@gmail.com>
> # Date 1696647746 -7200
> # Sat Oct 07 05:02:26 2023 +0200
> # Node ID ab14ea51bbb15331c9f44f14901d0fd378168647
> # Parent c073e545e1cdcc736f8869a012a78b2dd836eac9
> optimize the code for searching file in ngx_conf_open_file
>
> This patch combines two consecutive if statements into one and leveraging
> short circuits circuiting. In the current code, we check the lengths of
> file names and if they match, the file names are being compared.
>
> I see few other places in the code that writing multiple if statements are
> preferred over short circuit evaluation (e.g.
> http://hg.nginx.org/nginx/file/tip/src/http/ngx_http_file_cache.c#l1153). I
> wanted to make sure if it's a matter of community's taste or if it's in
> line with some performance considerations?
>
> diff -r c073e545e1cd -r ab14ea51bbb1 src/core/ngx_conf_file.c
> --- a/src/core/ngx_conf_file.c Thu May 18 23:42:22 2023 +0200
> +++ b/src/core/ngx_conf_file.c Sat Oct 07 05:02:26 2023 +0200
> @@ -927,11 +927,7 @@
> i = 0;
> }
>
> - if (full.len != file[i].name.len) {
> - continue;
> - }
> -
> - if (ngx_strcmp(full.data, file[i].name.data) == 0) {
> + if (full.len == file[i].name.len && ngx_strcmp(full.data,
> file[i].name.data) == 0) {
> return &file[i];
> }
> }

Thanks for your efforts, but we prefer the code as is.

There is no difference between these variants from the performance
point of view. On the other hand, the existing variant with
separate length comparison is certainly more readable. Further,
the suggested change uses incorrect indentation, which is just
wrong.

For more information about submitting patches, please see tips
here:

http://nginx.org/en/docs/contributing_changes.html

For more information about nginx coding style, please refer to the
code and here:

http://nginx.org/en/docs/dev/development_guide.html#code_style

Hope this helps.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel