{"thread":{"id":"6162","subject":"[PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","startedAt":"2006-12-31T02:02:18Z","lastAt":"2006-12-31T05:47:41Z","messageCount":7,"participants":["Shawn O. Pearce","Junio C Hamano","Shawn Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"30530","messageId":"20061231020218.GA5366@spearce.org","threadId":"6162","inReplyTo":null,"subject":"[PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-31T02:02:18Z","receivedAt":"2006-12-31T02:02:18Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sometimes its necessary to supply a value as a power of two in a\nconfiguration parameter. In this case the user may want to use\nthe standard suffixes such as K, KB, KiB, etc. to indicate that\nthe numerical value should be multiplied by a constant base before\nbeing used.\n\nThe new git_config_datasize function can be used in config file\nhandler functions to obtain a size_t in bytes.\n\nSigned-off-by: Shawn O. Pearce \n---\n\n Applies on top of sp/mmap, but was broken out to make it easier\n to apply earlier in case someone else needed this function.\n\n cache.h | 1 +\n config.c | 25 +++++++++++++++++++++++++\n 2 files changed, 26 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex a5fc232..abbcab3 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -418,6 +418,7 @@ extern int git_config_from_file(config_fn_t fn, const char *);\n extern int git_config(config_fn_t fn);\n extern int git_config_int(const char *, const char *);\n extern int git_config_bool(const char *, const char *);\n+extern size_t git_config_datasize(const char *, const char *);\n extern int git_config_set(const char *, const char *);\n extern int git_config_set_multivar(const char *, const char *, const char *, int);\n extern int git_config_rename_section(const char *, const char *);\ndiff --git a/config.c b/config.c\nindex 2e0d5a8..07ad2f1 100644\n--- a/config.c\n+++ b/config.c\n@@ -255,6 +255,31 @@ int git_config_bool(const char *name, const char *value)\n \treturn git_config_int(name, value) != 0;\n }\n \n+size_t git_config_datasize(const char *name, const char *value)\n+{\n+\tif (value && *value) {\n+\t\tchar *end;\n+\t\tunsigned long val = strtoul(value, &end, 0);\n+\t\twhile (isspace(*end))\n+\t\t\tend++;\n+\t\tif (!*end)\n+\t\t\treturn val;\n+\t\tif (!strcasecmp(end, \"k\")\n+\t\t\t|| !strcasecmp(end, \"kb\")\n+\t\t\t|| !strcasecmp(end, \"kib\"))\n+\t\t\treturn val * 1024;\n+\t\tif (!strcasecmp(end, \"m\")\n+\t\t\t|| !strcasecmp(end, \"mb\")\n+\t\t\t|| !strcasecmp(end, \"mib\"))\n+\t\t\treturn val * 1024 * 1024;\n+\t\tif (!strcasecmp(end, \"g\")\n+\t\t\t|| !strcasecmp(end, \"gb\")\n+\t\t\t|| !strcasecmp(end, \"gib\"))\n+\t\t\treturn val * 1024 * 1024 * 1024;\n+\t}\n+\tdie(\"bad config value for '%s' in %s\", name, config_file_name);\n+}\n+\n int git_default_config(const char *var, const char *value)\n {\n \t/* This needs a better name */\n-- \n1.5.0.rc0.g6bb1\n"},{"id":"30535","messageId":"7vodpkhjzq.fsf@assigned-by-dhcp.cox.net","threadId":"6162","inReplyTo":"20061231020218.GA5366@spearce.org","subject":"Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-31T02:33:45Z","receivedAt":"2006-12-31T02:33:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Sometimes its necessary to supply a value as a power of two in a\n> configuration parameter. In this case the user may want to use\n> the standard suffixes such as K, KB, KiB, etc. to indicate that\n> the numerical value should be multiplied by a constant base before\n> being used.\n\nIf you are really going to do this, I think we would need\nsomething similar to --bool to repo-config command.\n\nAlso can we fix the definition of core.packedGitWindowSize to be\nindependent of the page size on a particular platform?\n"},{"id":"30536","messageId":"20061231023809.GC5082@spearce.org","threadId":"6162","inReplyTo":"7vodpkhjzq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-31T02:38:09Z","receivedAt":"2006-12-31T02:38:09Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> \n> > Sometimes its necessary to supply a value as a power of two in a\n> > configuration parameter. In this case the user may want to use\n> > the standard suffixes such as K, KB, KiB, etc. to indicate that\n> > the numerical value should be multiplied by a constant base before\n> > being used.\n> \n> If you are really going to do this, I think we would need\n> something similar to --bool to repo-config command.\n\nThat shouldn't be difficult.\n \n> Also can we fix the definition of core.packedGitWindowSize to be\n> independent of the page size on a particular platform?\n\nWhat do you mean? mmap() only works in page units, and because of\nthe way the code is built the minimum size we can allow is 2 pages.\n\nAsking mmap() to map less than a full page in the last page of\na given window is silly, as that is just going to waste virtual\naddress space or cause pain for the OS, depending on how that\ngets implemented.\n\n-- \nShawn.\n"},{"id":"30540","messageId":"7vk608hiwq.fsf@assigned-by-dhcp.cox.net","threadId":"6162","inReplyTo":"20061231023809.GC5082@spearce.org","subject":"Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-31T02:57:09Z","receivedAt":"2006-12-31T02:57:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n>> Also can we fix the definition of core.packedGitWindowSize to be\n>> independent of the page size on a particular platform?\n>\n> What do you mean? mmap() only works in page units, and because of\n> the way the code is built the minimum size we can allow is 2 pages.\n>\n> Asking mmap() to map less than a full page in the last page of\n> a given window is silly, as that is just going to waste virtual\n> address space or cause pain for the OS, depending on how that\n> gets implemented.\n\nAh, I misread the code. I thought the value is integer that\nstates number of pages, but you are only rounding it down and\nenforcing two-page minimum.\n\nSorry for the noise.\n"},{"id":"30541","messageId":"20061231030119.GD5082@spearce.org","threadId":"6162","inReplyTo":"7vk608hiwq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-31T03:01:19Z","receivedAt":"2006-12-31T03:01:19Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Shawn Pearce writes:\n> \n> >> Also can we fix the definition of core.packedGitWindowSize to be\n> >> independent of the page size on a particular platform?\n> >\n> > What do you mean? mmap() only works in page units, and because of\n> > the way the code is built the minimum size we can allow is 2 pages.\n> >\n> > Asking mmap() to map less than a full page in the last page of\n> > a given window is silly, as that is just going to waste virtual\n> > address space or cause pain for the OS, depending on how that\n> > gets implemented.\n> \n> Ah, I misread the code. I thought the value is integer that\n> states number of pages, but you are only rounding it down and\n> enforcing two-page minimum.\n\nSo in your test case where you thought you entered 20 pages, what you\nreally had was just 2 pages. We rounded 20 bytes down to 0 pages,\nthen forced that to a minimum of 2 pages. :-)\n\n-- \nShawn.\n"},{"id":"30554","messageId":"7vejqgk4l1.fsf@assigned-by-dhcp.cox.net","threadId":"6162","inReplyTo":"20061231030119.GD5082@spearce.org","subject":"Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-31T05:38:18Z","receivedAt":"2006-12-31T05:38:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> So in your test case where you thought you entered 20 pages, what you\n> really had was just 2 pages. We rounded 20 bytes down to 0 pages,\n> then forced that to a minimum of 2 pages. :-)\n\nYes.\n\nAnd I found something else that is interesting. I still had 2\npages windows but reduced packedgitlimit to 256kB and re-run the\nsame test to reproduce all merges in git.git. It still did not\nfail, but it took even shorter wallclock time (again, it is only\n3 seconds or so out of 5 1/2 minuts, so it is probably\nstatistically insignificant).\n"},{"id":"30556","messageId":"20061231054741.GA6106@spearce.org","threadId":"6162","inReplyTo":"7vejqgk4l1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-31T05:47:41Z","receivedAt":"2006-12-31T05:47:41Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Shawn Pearce writes:\n> \n> > So in your test case where you thought you entered 20 pages, what you\n> > really had was just 2 pages. We rounded 20 bytes down to 0 pages,\n> > then forced that to a minimum of 2 pages. :-)\n> \n> Yes.\n> \n> And I found something else that is interesting. I still had 2\n> pages windows but reduced packedgitlimit to 256kB and re-run the\n> same test to reproduce all merges in git.git. It still did not\n> fail, but it took even shorter wallclock time (again, it is only\n> 3 seconds or so out of 5 1/2 minuts, so it is probably\n> statistically insignificant).\n\nIts possible the packed_git.windows list is hurting us here.\nIts O(n) to locate a given window. The larger the ratio between\npackedGitWindowSize and packedGitLimit the longer that list can get,\nand the longer it can take to locate a window.\n\nMaybe that should be a simple red-black tree?\n\nMore testing probably needs to be done though.\n\n-- \nShawn.\n"}]}