# Dubious diff using --diff-algorithm=histogram 3 messages from 2023-04-04 to 2023-04-28. Participants: Alejandro Colomar. Thread: https://gitlist.dev/t/59541 ## Alejandro Colomar, 2023-04-04 19:51 Subject: Dubious diff using --diff-algorithm=histogram Message-ID: URL: https://gitlist.dev/e/a0b6d751-25fd-a132-f8a5-9844e5d9b8a6%40gmail.com ``` Hi, I've met some strange diff with the histogram algorithm. I know this has a lot of magic in it, and it's probably not so trivial to improve, and maybe my code base is so weird that it makes the histogram do crazy stuff, but I wanted to show it to you in case there's anything that can be improved. So, patience is showing a better diff than histogram, which shouldn't happen usually, and less in this case, where p++ is such a common thing to do (although maybe this nxt_slow_path() macro is called a lot in the file and confuses the histogram?). Code base: Cheers, Alex --- alx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=histogram diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 8646a6ce..92bdfb1f 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos, space_after_target: - if (nxt_slow_path(end - p < 10)) { - p++; + if (nxt_slow_path(end - p < 10)) { + if (p == end) { return NXT_AGAIN; } alx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=patience diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 8646a6ce..92bdfb1f 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos, space_after_target: + p++; + if (nxt_slow_path(end - p < 10)) { - p++; - if (p == end) { return NXT_AGAIN; } alx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=myers diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 8646a6ce..92bdfb1f 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos, space_after_target: - if (nxt_slow_path(end - p < 10)) { - p++; + if (nxt_slow_path(end - p < 10)) { + if (p == end) { return NXT_AGAIN; } alx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=minimal diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 8646a6ce..92bdfb1f 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos, space_after_target: - if (nxt_slow_path(end - p < 10)) { - p++; + if (nxt_slow_path(end - p < 10)) { + if (p == end) { return NXT_AGAIN; } -- GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5 ``` ## Alejandro Colomar, 2023-04-04 19:53 Subject: Re: Dubious diff using --diff-algorithm=histogram Message-ID: <0f90f1a0-56bc-7ca4-e276-9c163ec8ec2c@gmail.com> URL: https://gitlist.dev/e/0f90f1a0-56bc-7ca4-e276-9c163ec8ec2c%40gmail.com In-Reply-To: ``` On 4/4/23 21:51, Alejandro Colomar wrote: > Hi, > > I've met some strange diff with the histogram algorithm. I know this > has a lot of magic in it, and it's probably not so trivial to improve, > and maybe my code base is so weird that it makes the histogram do crazy > stuff, but I wanted to show it to you in case there's anything that can > be improved. > > So, patience is showing a better diff than histogram, which shouldn't > happen usually, and less in this case, where p++ is such a common thing > to do (although maybe this nxt_slow_path() macro is called a lot in the > file and confuses the histogram?). > > Code base: > > Cheers, > Alex Now this has surprised me even more: alx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=histogram diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 8646a6ce..3559f2e2 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -340,9 +340,9 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos, space_after_target: - if (nxt_slow_path(end - p < 10)) { + p++; - p++; + if (nxt_slow_path(end - p < 10)) { if (p == end) { return NXT_AGAIN; alx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=patience diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 8646a6ce..3559f2e2 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos, space_after_target: + p++; + if (nxt_slow_path(end - p < 10)) { - p++; - if (p == end) { return NXT_AGAIN; } -- GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5 ``` ## Alejandro Colomar, 2023-04-28 21:36 Subject: Re: Dubious diff using --diff-algorithm=histogram Message-ID: URL: https://gitlist.dev/e/cb3a07d6-8441-b7fb-4994-3b22e3b974de%40gmail.com In-Reply-To: ``` Here's another example of a dubious diff using histogram: @@ -50,8 +65,11 @@ .SH Examples nxt_unit_ctx_t *ctx; nxt_unit_init_t init; - bzero(&init, sizeof(nxt_unit_init_t)); + if (argc == 3 && strcmp(argv[1], "-t") == 0) { + nthreads = atoi(argv[2]); + } + bzero(&init, sizeof(nxt_unit_init_t)); init.callbacks.request_handler = &request_handler; init.callbacks.ready_handler = &ready_handler; That call to bzero(3) is the only one in the entire file. patience shows this instead: @@ -50,8 +65,11 @@ .SH Examples nxt_unit_ctx_t *ctx; nxt_unit_init_t init; + if (argc == 3 && strcmp(argv[1], "-t") == 0) { + nthreads = atoi(argv[2]); + } + bzero(&init, sizeof(nxt_unit_init_t)); - init.callbacks.request_handler = &request_handler; init.callbacks.ready_handler = &ready_handler; -- GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5 ```