{"thread":{"id":"61784","subject":"[PATCH] Fix to avoid high memory footprint","startedAt":"2024-07-16T08:04:03Z","lastAt":"2024-07-31T13:34:03Z","messageCount":15,"participants":["Haritha via GitGitGadget","Jeff King","Junio C Hamano","Torsten Bögershausen","Haritha D"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"498779","messageId":"pull.1744.git.git.1721117039874.gitgitgadget@gmail.com","threadId":"61784","inReplyTo":null,"subject":"[PATCH] Fix to avoid high memory footprint","fromName":"Haritha via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-16T08:03:59Z","receivedAt":"2024-07-16T08:04:03Z","isPatch":true,"sender":{"key":"name:Haritha","avatar":null},"body":"From: D Harithamma \n\nThis fix avoids high memory footprint when\nadding files that require conversion.\nGit has a trace_encoding routine that prints trace\noutput when GIT_TRACE_WORKING_TREE_ENCODING=1 is\nset. This environment variable is used to debug\nthe encoding contents.\nWhen a 40MB file is added, it requests close to\n1.8GB of storage from xrealloc which can lead\nto out of memory errors.\nHowever, the check for\nGIT_TRACE_WORKING_TREE_ENCODING is done after\nthe string is allocated. This resolves high\nmemory footprints even when\nGIT_TRACE_WORKING_TREE_ENCODING is not active.\nThis fix adds an early exit to avoid the\nunnecessary memory allocation.\n\nSigned-off-by: Haritha D \n---\n Fix to avoid high memory footprint\n \n This fix avoids high memory footprint when adding files that require\n conversion\n \n Git has a trace_encoding routine that prints trace output when\n GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment variable is\n used to debug the encoding contents. When a 40MB file is added, it\n requests close to 1.8GB of storage from xrealloc which can lead to out\n of memory errors. However, the check for GIT_TRACE_WORKING_TREE_ENCODING\n is done after the string is allocated. This resolves high memory\n footprints even when GIT_TRACE_WORKING_TREE_ENCODING is not active. This\n fix adds an early exit to avoid the unnecessary memory allocation.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1744%2FHarithaIBM%2FmemFootprintFix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1744/HarithaIBM/memFootprintFix-v1\nPull-Request: https://github.com/git/git/pull/1744\n\n convert.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2d..e765bcd53d6 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -324,6 +324,11 @@ static void trace_encoding(const char *context, const char *path,\n \tstruct strbuf trace = STRBUF_INIT;\n \tint i;\n \n+\t// If tracing is not on, exit early to avoid high memory footprint\n+\tif (!trace_pass_fl(&coe)) {\n+\t\treturn;\n+\t}\n+\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n\nbase-commit: 557ae147e6cdc9db121269b058c757ac5092f9c9\n-- \ngitgitgadget\n"},{"id":"498815","messageId":"20240717061651.GE547635@coredump.intra.peff.net","threadId":"61784","inReplyTo":"pull.1744.git.git.1721117039874.gitgitgadget@gmail.com","subject":"Re: [PATCH] Fix to avoid high memory footprint","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-17T06:16:51Z","receivedAt":"2024-07-17T06:16:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 16, 2024 at 08:03:59AM +0000, Haritha via GitGitGadget wrote:\n\n> From: D Harithamma \n> \n> This fix avoids high memory footprint when\n> adding files that require conversion.\n> Git has a trace_encoding routine that prints trace\n> output when GIT_TRACE_WORKING_TREE_ENCODING=1 is\n> set. This environment variable is used to debug\n> the encoding contents.\n> When a 40MB file is added, it requests close to\n> 1.8GB of storage from xrealloc which can lead\n> to out of memory errors.\n> However, the check for\n> GIT_TRACE_WORKING_TREE_ENCODING is done after\n> the string is allocated. This resolves high\n> memory footprints even when\n> GIT_TRACE_WORKING_TREE_ENCODING is not active.\n> This fix adds an early exit to avoid the\n> unnecessary memory allocation.\n> \n> Signed-off-by: Haritha D \n\nGood find. Any trace function should verify that tracing is enabled\nbefore doing any substantial work.\n\nLet's take a look at your patch. First, your line wrapping is unusual,\nmaking the commit message a bit hard to read. We'd usually shoot for ~72\ncharacters per line. So more like:\n\n> This fix avoids high memory footprint when adding files that require\n> conversion. Git has a trace_encoding routine that prints trace output\n> when GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment\n> variable is used to debug the encoding contents. When a 40MB file is\n> added, it requests close to 1.8GB of storage from xrealloc which can\n> lead to out of memory errors. However, the check for\n> GIT_TRACE_WORKING_TREE_ENCODING is done after the string is allocated.\n> This resolves high memory footprints even when\n> GIT_TRACE_WORKING_TREE_ENCODING is not active. This fix adds an early\n> exit to avoid the unnecessary memory allocation.\n\nSecond, we'd like a full real name in the Signed-off-by line, as you're\nagreeing to the DCO. See:\n\n https://git-scm.com/docs/SubmittingPatches#sign-off\n\nLikewise, the author name should match the signoff name (you can use\n\"git commit --amend --author=...\" to fix it).\n\nFor the patch itself:\n\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -324,6 +324,11 @@ static void trace_encoding(const char *context, const char *path,\n> \tstruct strbuf trace = STRBUF_INIT;\n> \tint i;\n> \n> +\t// If tracing is not on, exit early to avoid high memory footprint\n> +\tif (!trace_pass_fl(&coe)) {\n> +\t\treturn;\n> +\t}\n\nI don't think trace_pass_fl() is what you want. It will return true if\nthe trace fd is non-zero (so tracing was requested), but also if the key\nhas not yet been initialized (i.e., nobody has used this key to try\nprinting anything yet).\n\nI think you'd just use trace_want(&coe) instead.\n\nAlso, two style nits:\n\n - our usual style (see Documentation/CodingGuidelines) is to avoid\n braces for one-liners.\n\n - we only use the /* */ comment form, not //. Though IMHO you could\n skip the comment completely here, as an early-return check in a\n tracing function is pretty obvious.\n\nIt would be nice if we could test this, but besides the wasted work, I\ndon't think there's any user-visible behavior (the problem is that we\nare computing things when we're _not_ tracing, so there's nothing for\nthe user to see). And there's no provision in our test suite for\nmeasuring memory usage of a program. So I think we can live without it,\nand just manually verifying that it works (but it would be good to show\nthe measurements you did manually in the commit message).\n\n-Peff\n"},{"id":"499253","messageId":"pull.1744.v2.git.git.1721821503173.gitgitgadget@gmail.com","threadId":"61784","inReplyTo":"pull.1744.git.git.1721117039874.gitgitgadget@gmail.com","subject":"[PATCH v2] Fix to avoid high memory footprint","fromName":"Haritha via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-24T11:45:03Z","receivedAt":"2024-07-24T11:45:07Z","isPatch":true,"sender":{"key":"name:Haritha","avatar":null},"body":"From: D Harithamma \n\nThis fix avoids high memory footprint when adding files that require\nconversion. Git has a trace_encoding routine that prints trace output\nwhen GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment\nvariable is used to debug the encoding contents. When a 40MB file is\nadded, it requests close to 1.8GB of storage from xrealloc which can\nlead to out of memory errors. However, the check for\nGIT_TRACE_WORKING_TREE_ENCODING is done after the string is allocated.\nThis resolves high memory footprints even when\nGIT_TRACE_WORKING_TREE_ENCODING is not active. This fix adds an early\nexit to avoid the unnecessary memory allocation.\n\nSigned-off-by: Harithamma D \n---\n Fix to avoid high memory footprint\n \n This fix avoids high memory footprint when adding files that require\n conversion\n \n Git has a trace_encoding routine that prints trace output when\n GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment variable is\n used to debug the encoding contents. When a 40MB file is added, it\n requests close to 1.8GB of storage from xrealloc which can lead to out\n of memory errors. However, the check for GIT_TRACE_WORKING_TREE_ENCODING\n is done after the string is allocated. This resolves high memory\n footprints even when GIT_TRACE_WORKING_TREE_ENCODING is not active. This\n fix adds an early exit to avoid the unnecessary memory allocation.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1744%2FHarithaIBM%2FmemFootprintFix-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1744/HarithaIBM/memFootprintFix-v2\nPull-Request: https://github.com/git/git/pull/1744\n\nRange-diff vs v1:\n\n 1: 51c02f58fd6 ! 1: 500b7eacf2a Fix to avoid high memory footprint\n @@ Metadata\n ## Commit message ##\n Fix to avoid high memory footprint\n \n - This fix avoids high memory footprint when\n - adding files that require conversion.\n - Git has a trace_encoding routine that prints trace\n - output when GIT_TRACE_WORKING_TREE_ENCODING=1 is\n - set. This environment variable is used to debug\n - the encoding contents.\n - When a 40MB file is added, it requests close to\n - 1.8GB of storage from xrealloc which can lead\n - to out of memory errors.\n - However, the check for\n - GIT_TRACE_WORKING_TREE_ENCODING is done after\n - the string is allocated. This resolves high\n - memory footprints even when\n - GIT_TRACE_WORKING_TREE_ENCODING is not active.\n - This fix adds an early exit to avoid the\n - unnecessary memory allocation.\n + This fix avoids high memory footprint when adding files that require\n + conversion. Git has a trace_encoding routine that prints trace output\n + when GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment\n + variable is used to debug the encoding contents. When a 40MB file is\n + added, it requests close to 1.8GB of storage from xrealloc which can\n + lead to out of memory errors. However, the check for\n + GIT_TRACE_WORKING_TREE_ENCODING is done after the string is allocated.\n + This resolves high memory footprints even when\n + GIT_TRACE_WORKING_TREE_ENCODING is not active. This fix adds an early\n + exit to avoid the unnecessary memory allocation.\n \n - Signed-off-by: Haritha D \n + Signed-off-by: Harithamma D \n \n ## convert.c ##\n @@ convert.c: static void trace_encoding(const char *context, const char *path,\n \tstruct strbuf trace = STRBUF_INIT;\n \tint i;\n \n -+\t// If tracing is not on, exit early to avoid high memory footprint\n -+\tif (!trace_pass_fl(&coe)) {\n ++\tif (!trace_want(&coe))\n +\t\treturn;\n -+\t}\n +\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n\n\n convert.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2d..c4ddc4de81b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -324,6 +324,9 @@ static void trace_encoding(const char *context, const char *path,\n \tstruct strbuf trace = STRBUF_INIT;\n \tint i;\n \n+\tif (!trace_want(&coe))\n+\t\treturn;\n+\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n\nbase-commit: 557ae147e6cdc9db121269b058c757ac5092f9c9\n-- \ngitgitgadget\n"},{"id":"499302","messageId":"xmqqmsm6sc0q.fsf@gitster.g","threadId":"61784","inReplyTo":"pull.1744.v2.git.git.1721821503173.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] Fix to avoid high memory footprint","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-24T21:41:57Z","receivedAt":"2024-07-24T21:42:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Haritha via GitGitGadget\" writes:\n\n> From: D Harithamma \n>\n> This fix avoids high memory footprint when adding files that require\n> conversion. Git has a trace_encoding routine that prints trace output\n> when GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment\n> variable is used to debug the encoding contents. When a 40MB file is\n> added, it requests close to 1.8GB of storage from xrealloc which can\n> lead to out of memory errors. However, the check for\n> GIT_TRACE_WORKING_TREE_ENCODING is done after the string is allocated.\n> This resolves high memory footprints even when\n> GIT_TRACE_WORKING_TREE_ENCODING is not active. This fix adds an early\n> exit to avoid the unnecessary memory allocation.\n\nThe sentences jump around and the logic flow is hard to follow. The\nfirst sentence makes a claim of what it does (but the readers have\nnot bee told where that problem comes from). The second sentence\nmakes a statement of a fact, but the readers do not yet know at that\npoint what relevance the fact has to the issue at hand, etc.\n\nThe usual way to compose a log message of this project is to\n\n - Give an observation on how the current system work in the present\n tense (so no need to say \"Currently X is Y\", just \"X is Y\"), and\n discuss what you perceive as a problem in it.\n\n - Propose a solution (optional---often, problem description\n trivially leads to an obvious solution in reader's minds).\n\n - Give commands to the codebase to \"become like so\".\n\nin this order.\n\n\n When Git needs to add a file that require encoding conversion,\n but tracing of encoding conversion is *not* requested via\n setting GIT_TRACE_WORKING_TREE_ENCODING environment variable,\n the trace_encoding() function still allocated and prepared\n \"human readable\" copies of the file contents before and after\n conversion to show in the trace. This wasted a lot of memory\n footprint and runtime cycles without giving any user-visible\n benefit.\n\n Exit early from the function when we we are not tracing before\n we spend all the effort, not after.\n\nor something, perhaps?\n\nI am wondering if we should be able to test this, but \"git grep\nGIT_TRACE_WORKING_TREE_ENCODING t/\" is not finding any existing test\nin the area.\n\n> Signed-off-by: Harithamma D \n\nThis does not match the \"From: \" line above. Please pick one way to\nspell your name and identify yourself to this project, and use it\nconsistently.\n\nThanks.\n\n> diff --git a/convert.c b/convert.c\n> index d8737fe0f2d..c4ddc4de81b 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -324,6 +324,9 @@ static void trace_encoding(const char *context, const char *path,\n> \tstruct strbuf trace = STRBUF_INIT;\n> \tint i;\n> \n> +\tif (!trace_want(&coe))\n> +\t\treturn;\n> +\n\nThe actual fix is so simple and nice ;-)\n"},{"id":"499304","messageId":"20240724221638.GA568325@coredump.intra.peff.net","threadId":"61784","inReplyTo":"pull.1744.v2.git.git.1721821503173.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] Fix to avoid high memory footprint","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-24T22:16:38Z","receivedAt":"2024-07-24T22:16:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 24, 2024 at 11:45:03AM +0000, Haritha via GitGitGadget wrote:\n\n> diff --git a/convert.c b/convert.c\n> index d8737fe0f2d..c4ddc4de81b 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -324,6 +324,9 @@ static void trace_encoding(const char *context, const char *path,\n> \tstruct strbuf trace = STRBUF_INIT;\n> \tint i;\n> \n> +\tif (!trace_want(&coe))\n> +\t\treturn;\n> +\n> \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n> \tfor (i = 0; i < len && buf; ++i) {\n> \t\tstrbuf_addf(\n\nThe patch itself looks good. I confirmed that running:\n\n git init\n dd if=/dev/zero of=foo.bin bs=1M count=50\n echo '*.bin working-tree-encoding=UTF-16LE' >.gitattributes\n valgrind --tool=massif git add .\n\ngoes from a max heap of 1.7G down to 51MB with your patch (whereas I\nthink with the previous iteration it would not have, since the old check\ndid the wrong thing on the first call to trace_encoding()).\n\n-Peff\n"},{"id":"499393","messageId":"pull.1744.v3.git.git.1721975234873.gitgitgadget@gmail.com","threadId":"61784","inReplyTo":"pull.1744.v2.git.git.1721821503173.gitgitgadget@gmail.com","subject":"[PATCH v3] Fix to avoid high memory footprint","fromName":"Haritha via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-26T06:27:14Z","receivedAt":"2024-07-26T06:27:18Z","isPatch":true,"sender":{"key":"name:Haritha","avatar":null},"body":"From: D Harithamma \n\nWhen Git adds a file requiring encoding conversion and tracing of encoding\nconversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\nenvironment variable, the `trace_encoding()` function still allocates &\nprepares \"human readable\" copies of the file contents before and after\nconversion to show in the trace. This results in a high memory footprint\nand increased runtime without providing any user-visible benefit.\n\nThis fix introduces an early exit from the `trace_encoding()` function\nwhen tracing is not requested, preventing unnecessary memory allocation\nand processing.\n\nSigned-off-by: Harithamma D \n---\n Fix to avoid high memory footprint\n \n This fix avoids high memory footprint when adding files that require\n conversion\n \n Git has a trace_encoding routine that prints trace output when\n GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment variable is\n used to debug the encoding contents. When a 40MB file is added, it\n requests close to 1.8GB of storage from xrealloc which can lead to out\n of memory errors. However, the check for GIT_TRACE_WORKING_TREE_ENCODING\n is done after the string is allocated. This resolves high memory\n footprints even when GIT_TRACE_WORKING_TREE_ENCODING is not active. This\n fix adds an early exit to avoid the unnecessary memory allocation.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1744%2FHarithaIBM%2FmemFootprintFix-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1744/HarithaIBM/memFootprintFix-v3\nPull-Request: https://github.com/git/git/pull/1744\n\nRange-diff vs v2:\n\n 1: 500b7eacf2a ! 1: d864de64380 Fix to avoid high memory footprint\n @@ Metadata\n ## Commit message ##\n Fix to avoid high memory footprint\n \n - This fix avoids high memory footprint when adding files that require\n - conversion. Git has a trace_encoding routine that prints trace output\n - when GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment\n - variable is used to debug the encoding contents. When a 40MB file is\n - added, it requests close to 1.8GB of storage from xrealloc which can\n - lead to out of memory errors. However, the check for\n - GIT_TRACE_WORKING_TREE_ENCODING is done after the string is allocated.\n - This resolves high memory footprints even when\n - GIT_TRACE_WORKING_TREE_ENCODING is not active. This fix adds an early\n - exit to avoid the unnecessary memory allocation.\n + When Git adds a file requiring encoding conversion and tracing of encoding\n + conversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\n + environment variable, the `trace_encoding()` function still allocates &\n + prepares \"human readable\" copies of the file contents before and after\n + conversion to show in the trace. This results in a high memory footprint\n + and increased runtime without providing any user-visible benefit.\n +\n + This fix introduces an early exit from the `trace_encoding()` function\n + when tracing is not requested, preventing unnecessary memory allocation\n + and processing.\n \n Signed-off-by: Harithamma D \n \n\n\n convert.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2d..c4ddc4de81b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -324,6 +324,9 @@ static void trace_encoding(const char *context, const char *path,\n \tstruct strbuf trace = STRBUF_INIT;\n \tint i;\n \n+\tif (!trace_want(&coe))\n+\t\treturn;\n+\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n\nbase-commit: 557ae147e6cdc9db121269b058c757ac5092f9c9\n-- \ngitgitgadget\n"},{"id":"499410","messageId":"20240726095535.GA32544@tb-raspi4","threadId":"61784","inReplyTo":"pull.1744.v3.git.git.1721975234873.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] Fix to avoid high memory footprint","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-07-26T09:55:35Z","receivedAt":"2024-07-26T09:55:51Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Fri, Jul 26, 2024 at 06:27:14AM +0000, Haritha via GitGitGadget wrote:\n> From: D Harithamma \n>\n> When Git adds a file requiring encoding conversion and tracing of encoding\n> conversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\n> environment variable, the `trace_encoding()` function still allocates &\n> prepares \"human readable\" copies of the file contents before and after\n> conversion to show in the trace. This results in a high memory footprint\n> and increased runtime without providing any user-visible benefit.\n>\n> This fix introduces an early exit from the `trace_encoding()` function\n> when tracing is not requested, preventing unnecessary memory allocation\n> and processing.\n>\n> Signed-off-by: Harithamma D \n> ---\n> Fix to avoid high memory footprint\n>\n\nThis head line\n> Fix to avoid high memory footprint\ndoes not tell to much when and how it happens.\nThe word \"fix\" is not realy needed (in this project).\n\nSomething like\n \"convert: avoid high memory footprint\"\n\nwill tell the reader, that only the convert functionality is affected\nby this patch.\n\nThinking about it, another suggestion may be:\n\nconvert: Reduce memory allocation when trace_encoding() is not used\n\nIf someone browses through the whole history of Git, this is easier to\nfollow.\n\nThe exact wording may be improved, important would be to have \"convert:\"\n\nas the first keyword, and then \"memory allocation\" and \"trace_encoding()\"\ngive hints, what this is all about in one line.\n\nAnd the rest looks good.\n"},{"id":"499443","messageId":"pull.1744.v4.git.git.1722002432630.gitgitgadget@gmail.com","threadId":"61784","inReplyTo":"pull.1744.v3.git.git.1721975234873.gitgitgadget@gmail.com","subject":"[PATCH v4] convert: avoid high memory footprint","fromName":"Haritha via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-26T14:00:32Z","receivedAt":"2024-07-26T14:00:35Z","isPatch":true,"sender":{"key":"name:Haritha","avatar":null},"body":"From: D Harithamma \n\nWhen Git adds a file requiring encoding conversion and tracing of encoding\nconversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\nenvironment variable, the `trace_encoding()` function still allocates &\nprepares \"human readable\" copies of the file contents before and after\nconversion to show in the trace. This results in a high memory footprint\nand increased runtime without providing any user-visible benefit.\n\nThis fix introduces an early exit from the `trace_encoding()` function\nwhen tracing is not requested, preventing unnecessary memory allocation\nand processing.\n\nSigned-off-by: Harithamma D \n---\n Fix to avoid high memory footprint\n \n This fix avoids high memory footprint when adding files that require\n conversion\n \n Git has a trace_encoding routine that prints trace output when\n GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment variable is\n used to debug the encoding contents. When a 40MB file is added, it\n requests close to 1.8GB of storage from xrealloc which can lead to out\n of memory errors. However, the check for GIT_TRACE_WORKING_TREE_ENCODING\n is done after the string is allocated. This resolves high memory\n footprints even when GIT_TRACE_WORKING_TREE_ENCODING is not active. This\n fix adds an early exit to avoid the unnecessary memory allocation.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1744%2FHarithaIBM%2FmemFootprintFix-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1744/HarithaIBM/memFootprintFix-v4\nPull-Request: https://github.com/git/git/pull/1744\n\nRange-diff vs v3:\n\n 1: d864de64380 ! 1: 50758a4fb94 Fix to avoid high memory footprint\n @@ Metadata\n Author: D Harithamma \n \n ## Commit message ##\n - Fix to avoid high memory footprint\n + convert: avoid high memory footprint\n \n When Git adds a file requiring encoding conversion and tracing of encoding\n conversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\n\n\n convert.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2d..c4ddc4de81b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -324,6 +324,9 @@ static void trace_encoding(const char *context, const char *path,\n \tstruct strbuf trace = STRBUF_INIT;\n \tint i;\n \n+\tif (!trace_want(&coe))\n+\t\treturn;\n+\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n\nbase-commit: 557ae147e6cdc9db121269b058c757ac5092f9c9\n-- \ngitgitgadget\n"},{"id":"499446","messageId":"xmqqed7gyyyd.fsf@gitster.g","threadId":"61784","inReplyTo":"pull.1744.v3.git.git.1721975234873.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] Fix to avoid high memory footprint","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-26T15:06:50Z","receivedAt":"2024-07-26T15:06:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Haritha via GitGitGadget\" writes:\n\n> From: D Harithamma \n> ...\n> Signed-off-by: Harithamma D \n\nI am assuming that you and the person who did d254e650 (build:\nsupport z/OS (OS/390)., 2024-03-06) are the same person? That\ncommit was signed off like so:\n\n commit d254e65092daba8667d6b4d5b4f59c099c1edd1f\n Author: Haritha D \n Date: Wed Mar 6 05:44:17 2024 +0000\n\n build: support z/OS (OS/390).\n\n Introduced z/OS (OS/390) as a platform in config.mak.uname\n\n Signed-off-by: Haritha D \n Signed-off-by: Junio C Hamano \n\nIt is OK if you really want to use a longer name this time, but then\nplease be consistent within a single commit. The author name of the\nproposed commit is \"D Harithamma\" and a different name \"Harithamma D\"\nis used to sign off the commit, which is not what we want to see.\n\nThanks.\n"},{"id":"499447","messageId":"xmqqa5i4yypc.fsf@gitster.g","threadId":"61784","inReplyTo":"pull.1744.v3.git.git.1721975234873.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] Fix to avoid high memory footprint","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-26T15:12:15Z","receivedAt":"2024-07-26T15:12:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Haritha via GitGitGadget\" writes:\n\nAnother thing.\n\n> Subject: Re: [PATCH v3] Fix to avoid high memory footprint\n\nThis does not tell us much about what area had problem under what\ncondition. \"git shortlog --no-merges -200 master\" may give us good\nexamples of how we typically write the title of our commits.\n\nIn the case of this change, ideally we should be able to tell that\nthis is about tracing the conversion codepath.\n\n Subject: [PATCH vN] encoding: return early when not tracing conversion\n\nor something, perhaps?\n"},{"id":"499549","messageId":"B292440D-119F-4D87-B308-EC90027EA63D@ibm.com","threadId":"61784","inReplyTo":"xmqqa5i4yypc.fsf@gitster.g","subject":"RE: [PATCH v3] Fix to avoid high memory footprint","fromName":"Haritha D","fromEmail":"harithamma.d@ibm.com","sentAt":"2024-07-30T03:41:17Z","receivedAt":"2024-07-30T03:41:34Z","isPatch":true,"sender":{"key":"harithamma.d@ibm.com","avatar":"https://avatars.githubusercontent.com/u/112700353?v=4"},"body":"Hello Team,\n\nI have retained convert as per Torsten's comment. Rest i have changed as per Junio's suggestion. \n\nThank you, everyone.\n\nOn 26/07/24, 8:42 PM, \"Junio C Hamano\" > wrote:\n\n\n\"Haritha via GitGitGadget\" > writes:\n\n\nAnother thing.\n\n\n> Subject: Re: [PATCH v3] Fix to avoid high memory footprint\n\n\nThis does not tell us much about what area had problem under what\ncondition. \"git shortlog --no-merges -200 master\" may give us good\nexamples of how we typically write the title of our commits.\n\n\nIn the case of this change, ideally we should be able to tell that\nthis is about tracing the conversion codepath.\n\n\nSubject: [PATCH vN] encoding: return early when not tracing conversion\n\n\nor something, perhaps?\n\n\n\n"},{"id":"499550","messageId":"pull.1744.v5.git.git.1722310937061.gitgitgadget@gmail.com","threadId":"61784","inReplyTo":"pull.1744.v4.git.git.1722002432630.gitgitgadget@gmail.com","subject":"[PATCH v5] convert: return early when not tracing","fromName":"Haritha via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-30T03:42:16Z","receivedAt":"2024-07-30T03:42:21Z","isPatch":true,"sender":{"key":"name:Haritha","avatar":null},"body":"From: D Harithamma \n\nWhen Git adds a file requiring encoding conversion and tracing of encoding\nconversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\nenvironment variable, the `trace_encoding()` function still allocates &\nprepares \"human readable\" copies of the file contents before and after\nconversion to show in the trace. This results in a high memory footprint\nand increased runtime without providing any user-visible benefit.\n\nThis fix introduces an early exit from the `trace_encoding()` function\nwhen tracing is not requested, preventing unnecessary memory allocation\nand processing.\n\nSigned-off-by: Harithamma D \n---\n Fix to avoid high memory footprint\n \n This fix avoids high memory footprint when adding files that require\n conversion\n \n Git has a trace_encoding routine that prints trace output when\n GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment variable is\n used to debug the encoding contents. When a 40MB file is added, it\n requests close to 1.8GB of storage from xrealloc which can lead to out\n of memory errors. However, the check for GIT_TRACE_WORKING_TREE_ENCODING\n is done after the string is allocated. This resolves high memory\n footprints even when GIT_TRACE_WORKING_TREE_ENCODING is not active. This\n fix adds an early exit to avoid the unnecessary memory allocation.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1744%2FHarithaIBM%2FmemFootprintFix-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1744/HarithaIBM/memFootprintFix-v5\nPull-Request: https://github.com/git/git/pull/1744\n\nRange-diff vs v4:\n\n 1: 50758a4fb94 ! 1: e2518b28f1c convert: avoid high memory footprint\n @@ Metadata\n Author: D Harithamma \n \n ## Commit message ##\n - convert: avoid high memory footprint\n + convert: return early when not tracing\n \n When Git adds a file requiring encoding conversion and tracing of encoding\n conversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\n\n\n convert.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2d..c4ddc4de81b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -324,6 +324,9 @@ static void trace_encoding(const char *context, const char *path,\n \tstruct strbuf trace = STRBUF_INIT;\n \tint i;\n \n+\tif (!trace_want(&coe))\n+\t\treturn;\n+\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n\nbase-commit: 557ae147e6cdc9db121269b058c757ac5092f9c9\n-- \ngitgitgadget\n"},{"id":"499660","messageId":"xmqqle1i8eov.fsf@gitster.g","threadId":"61784","inReplyTo":"pull.1744.v5.git.git.1722310937061.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] convert: return early when not tracing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-31T02:42:40Z","receivedAt":"2024-07-31T02:42:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Haritha via GitGitGadget\" writes:\n\n> From: D Harithamma \n>\n> When Git adds a file requiring encoding conversion and tracing of encoding\n> conversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\n> environment variable, the `trace_encoding()` function still allocates &\n> prepares \"human readable\" copies of the file contents before and after\n> conversion to show in the trace. This results in a high memory footprint\n> and increased runtime without providing any user-visible benefit.\n>\n> This fix introduces an early exit from the `trace_encoding()` function\n> when tracing is not requested, preventing unnecessary memory allocation\n> and processing.\n>\n> Signed-off-by: Harithamma D \n> ---\n\nIt seems that you forgot to adjust to\n\n https://lore.kernel.org/git/xmqqed7gyyyd.fsf@gitster.g/\n\nwhere I asked you to be consistent in the authorship name and sign\noff.\n\nFor now, as I like to allow \"git shortlog --author=...\" to group\ncontributions by a single author to a single bucket, I'll rewrite\nboth to the same name as used in d254e650 (build: support z/OS\n(OS/390)., 2024-03-06), but will not merge it down to 'next' before\nI hear what your response is.\n\nThanks.\n\n"},{"id":"499676","messageId":"F903ADA1-8F03-481E-9AC9-7ABFBE205D9D@ibm.com","threadId":"61784","inReplyTo":"xmqqle1i8eov.fsf@gitster.g","subject":"RE: [PATCH v5] convert: return early when not tracing","fromName":"Haritha D","fromEmail":"harithamma.d@ibm.com","sentAt":"2024-07-31T09:32:07Z","receivedAt":"2024-07-31T09:32:13Z","isPatch":true,"sender":{"key":"harithamma.d@ibm.com","avatar":"https://avatars.githubusercontent.com/u/112700353?v=4"},"body":"Hey Junio,\n\nYou're right. I forgot to respond to that question.\nI used full name based on Jeff's suggestion. \nI will ensure it remains consistent.\nThank you reminding me.\n\nBest Regards\nHaritha\n\nOn 31/07/24, 8:12 AM, \"Junio C Hamano\" > wrote:\n\n\n\"Haritha via GitGitGadget\" > writes:\n\n\n> From: D Harithamma >\n>\n> When Git adds a file requiring encoding conversion and tracing of encoding\n> conversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\n> environment variable, the `trace_encoding()` function still allocates &\n> prepares \"human readable\" copies of the file contents before and after\n> conversion to show in the trace. This results in a high memory footprint\n> and increased runtime without providing any user-visible benefit.\n>\n> This fix introduces an early exit from the `trace_encoding()` function\n> when tracing is not requested, preventing unnecessary memory allocation\n> and processing.\n>\n> Signed-off-by: Harithamma D >\n> ---\n\n\nIt seems that you forgot to adjust to\n\n\nhttps://lore.kernel.org/git/xmqqed7gyyyd.fsf@gitster.g/ \n\n\nwhere I asked you to be consistent in the authorship name and sign\noff.\n\n\nFor now, as I like to allow \"git shortlog --author=...\" to group\ncontributions by a single author to a single bucket, I'll rewrite\nboth to the same name as used in d254e650 (build: support z/OS\n(OS/390)., 2024-03-06), but will not merge it down to 'next' before\nI hear what your response is.\n\n\nThanks.\n\n\n\n\n\n"},{"id":"499689","messageId":"pull.1744.v6.git.git.1722432839473.gitgitgadget@gmail.com","threadId":"61784","inReplyTo":"pull.1744.v5.git.git.1722310937061.gitgitgadget@gmail.com","subject":"[PATCH v6] convert: return early when not tracing","fromName":"Haritha via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-31T13:33:59Z","receivedAt":"2024-07-31T13:34:03Z","isPatch":true,"sender":{"key":"name:Haritha","avatar":null},"body":"From: D Harithamma \n\nWhen Git adds a file requiring encoding conversion and tracing of encoding\nconversion is not requested via the GIT_TRACE_WORKING_TREE_ENCODING\nenvironment variable, the `trace_encoding()` function still allocates &\nprepares \"human readable\" copies of the file contents before and after\nconversion to show in the trace. This results in a high memory footprint\nand increased runtime without providing any user-visible benefit.\n\nThis fix introduces an early exit from the `trace_encoding()` function\nwhen tracing is not requested, preventing unnecessary memory allocation\nand processing.\n\nSigned-off-by: D Harithamma \n---\n Fix to avoid high memory footprint\n \n This fix avoids high memory footprint when adding files that require\n conversion\n \n Git has a trace_encoding routine that prints trace output when\n GIT_TRACE_WORKING_TREE_ENCODING=1 is set. This environment variable is\n used to debug the encoding contents. When a 40MB file is added, it\n requests close to 1.8GB of storage from xrealloc which can lead to out\n of memory errors. However, the check for GIT_TRACE_WORKING_TREE_ENCODING\n is done after the string is allocated. This resolves high memory\n footprints even when GIT_TRACE_WORKING_TREE_ENCODING is not active. This\n fix adds an early exit to avoid the unnecessary memory allocation.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1744%2FHarithaIBM%2FmemFootprintFix-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1744/HarithaIBM/memFootprintFix-v6\nPull-Request: https://github.com/git/git/pull/1744\n\nRange-diff vs v5:\n\n 1: e2518b28f1c ! 1: 7b68aa9de1d convert: return early when not tracing\n @@ Commit message\n when tracing is not requested, preventing unnecessary memory allocation\n and processing.\n \n - Signed-off-by: Harithamma D \n + Signed-off-by: D Harithamma \n \n ## convert.c ##\n @@ convert.c: static void trace_encoding(const char *context, const char *path,\n\n\n convert.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2d..c4ddc4de81b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -324,6 +324,9 @@ static void trace_encoding(const char *context, const char *path,\n \tstruct strbuf trace = STRBUF_INIT;\n \tint i;\n \n+\tif (!trace_want(&coe))\n+\t\treturn;\n+\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n\nbase-commit: 557ae147e6cdc9db121269b058c757ac5092f9c9\n-- \ngitgitgadget\n"}]}