{"thread":{"id":"59541","subject":"Dubious diff using --diff-algorithm=histogram","startedAt":"2023-04-04T19:51:34Z","lastAt":"2023-04-28T21:36:25Z","messageCount":3,"participants":["Alejandro Colomar"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"474802","messageId":"a0b6d751-25fd-a132-f8a5-9844e5d9b8a6@gmail.com","threadId":"59541","inReplyTo":null,"subject":"Dubious diff using --diff-algorithm=histogram","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-04-04T19:51:21Z","receivedAt":"2023-04-04T19:51:34Z","isPatch":false,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"Hi,\n\nI've met some strange diff with the histogram algorithm.  I know this\nhas a lot of magic in it, and it's probably not so trivial to improve,\nand maybe my code base is so weird that it makes the histogram do crazy\nstuff, but I wanted to show it to you in case there's anything that can\nbe improved.\n\nSo, patience is showing a better diff than histogram, which shouldn't\nhappen usually, and less in this case, where p++ is such a common thing\nto do (although maybe this nxt_slow_path() macro is called a lot in the\nfile and confuses the histogram?).\n\nCode base: <https://github.com/nginx/unit.git>\n\nCheers,\nAlex\n\n---\n\nalx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=histogram \ndiff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c\nindex 8646a6ce..92bdfb1f 100644\n--- a/src/nxt_http_parse.c\n+++ b/src/nxt_http_parse.c\n@@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos,\n \n space_after_target:\n \n-    if (nxt_slow_path(end - p < 10)) {\n-\n         p++;\n \n+    if (nxt_slow_path(end - p < 10)) {\n+\n         if (p == end) {\n             return NXT_AGAIN;\n         }\nalx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=patience \ndiff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c\nindex 8646a6ce..92bdfb1f 100644\n--- a/src/nxt_http_parse.c\n+++ b/src/nxt_http_parse.c\n@@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos,\n \n space_after_target:\n \n+        p++;\n+\n     if (nxt_slow_path(end - p < 10)) {\n \n-        p++;\n-\n         if (p == end) {\n             return NXT_AGAIN;\n         }\nalx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=myers \ndiff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c\nindex 8646a6ce..92bdfb1f 100644\n--- a/src/nxt_http_parse.c\n+++ b/src/nxt_http_parse.c\n@@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos,\n \n space_after_target:\n \n-    if (nxt_slow_path(end - p < 10)) {\n-\n         p++;\n \n+    if (nxt_slow_path(end - p < 10)) {\n+\n         if (p == end) {\n             return NXT_AGAIN;\n         }\nalx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=minimal \ndiff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c\nindex 8646a6ce..92bdfb1f 100644\n--- a/src/nxt_http_parse.c\n+++ b/src/nxt_http_parse.c\n@@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos,\n \n space_after_target:\n \n-    if (nxt_slow_path(end - p < 10)) {\n-\n         p++;\n \n+    if (nxt_slow_path(end - p < 10)) {\n+\n         if (p == end) {\n             return NXT_AGAIN;\n         }\n\n\n\n-- \n<http://www.alejandro-colomar.es/>\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"},{"id":"474803","messageId":"0f90f1a0-56bc-7ca4-e276-9c163ec8ec2c@gmail.com","threadId":"59541","inReplyTo":"a0b6d751-25fd-a132-f8a5-9844e5d9b8a6@gmail.com","subject":"Re: Dubious diff using --diff-algorithm=histogram","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-04-04T19:53:05Z","receivedAt":"2023-04-04T19:53:17Z","isPatch":false,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"On 4/4/23 21:51, Alejandro Colomar wrote:\n> Hi,\n> \n> I've met some strange diff with the histogram algorithm.  I know this\n> has a lot of magic in it, and it's probably not so trivial to improve,\n> and maybe my code base is so weird that it makes the histogram do crazy\n> stuff, but I wanted to show it to you in case there's anything that can\n> be improved.\n> \n> So, patience is showing a better diff than histogram, which shouldn't\n> happen usually, and less in this case, where p++ is such a common thing\n> to do (although maybe this nxt_slow_path() macro is called a lot in the\n> file and confuses the histogram?).\n> \n> Code base: <https://github.com/nginx/unit.git>\n> \n> Cheers,\n> Alex\n\nNow this has surprised me even more:\n\nalx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=histogram \ndiff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c\nindex 8646a6ce..3559f2e2 100644\n--- a/src/nxt_http_parse.c\n+++ b/src/nxt_http_parse.c\n@@ -340,9 +340,9 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos,\n \n space_after_target:\n \n-    if (nxt_slow_path(end - p < 10)) {\n+    p++;\n \n-        p++;\n+    if (nxt_slow_path(end - p < 10)) {\n \n         if (p == end) {\n             return NXT_AGAIN;\nalx@asus5775:~/src/nginx/unit/space$ git diff --diff-algorithm=patience \ndiff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c\nindex 8646a6ce..3559f2e2 100644\n--- a/src/nxt_http_parse.c\n+++ b/src/nxt_http_parse.c\n@@ -340,10 +340,10 @@ nxt_http_parse_request_line(nxt_http_request_parse_t *rp, u_char **pos,\n \n space_after_target:\n \n+    p++;\n+\n     if (nxt_slow_path(end - p < 10)) {\n \n-        p++;\n-\n         if (p == end) {\n             return NXT_AGAIN;\n         }\n\n\n-- \n<http://www.alejandro-colomar.es/>\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"},{"id":"476297","messageId":"cb3a07d6-8441-b7fb-4994-3b22e3b974de@gmail.com","threadId":"59541","inReplyTo":"a0b6d751-25fd-a132-f8a5-9844e5d9b8a6@gmail.com","subject":"Re: Dubious diff using --diff-algorithm=histogram","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-04-28T21:36:11Z","receivedAt":"2023-04-28T21:36:25Z","isPatch":false,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"Here's another example of a dubious diff using histogram:\n\n@@ -50,8 +65,11 @@ .SH Examples\n     nxt_unit_ctx_t   *ctx;\n     nxt_unit_init_t  init;\n \n-    bzero(&init, sizeof(nxt_unit_init_t));\n+    if (argc == 3 && strcmp(argv[1], \"-t\") == 0) {\n+        nthreads = atoi(argv[2]);\n+    }\n \n+    bzero(&init, sizeof(nxt_unit_init_t));\n     init.callbacks.request_handler = &request_handler;\n     init.callbacks.ready_handler = &ready_handler;\n \n\n\nThat call to bzero(3) is the only one in the entire file.\npatience shows this instead:\n\n\n@@ -50,8 +65,11 @@ .SH Examples\n     nxt_unit_ctx_t   *ctx;\n     nxt_unit_init_t  init;\n \n+    if (argc == 3 && strcmp(argv[1], \"-t\") == 0) {\n+        nthreads = atoi(argv[2]);\n+    }\n+\n     bzero(&init, sizeof(nxt_unit_init_t));\n-\n     init.callbacks.request_handler = &request_handler;\n     init.callbacks.ready_handler = &ready_handler;\n \n\n\n-- \n<http://www.alejandro-colomar.es/>\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"}]}