# [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. 7 messages from 2006-12-31 to 2006-12-31. Participants: Shawn O. Pearce, Junio C Hamano, Shawn Pearce. Thread: https://gitlist.dev/t/6162 ## Shawn O. Pearce, 2006-12-31 02:02 Subject: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. Message-ID: <20061231020218.GA5366@spearce.org> URL: https://gitlist.dev/e/20061231020218.GA5366%40spearce.org ``` Sometimes its necessary to supply a value as a power of two in a configuration parameter. In this case the user may want to use the standard suffixes such as K, KB, KiB, etc. to indicate that the numerical value should be multiplied by a constant base before being used. The new git_config_datasize function can be used in config file handler functions to obtain a size_t in bytes. Signed-off-by: Shawn O. Pearce --- Applies on top of sp/mmap, but was broken out to make it easier to apply earlier in case someone else needed this function. cache.h | 1 + config.c | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 0 deletions(-) diff --git a/cache.h b/cache.h index a5fc232..abbcab3 100644 --- a/cache.h +++ b/cache.h @@ -418,6 +418,7 @@ extern int git_config_from_file(config_fn_t fn, const char *); extern int git_config(config_fn_t fn); extern int git_config_int(const char *, const char *); extern int git_config_bool(const char *, const char *); +extern size_t git_config_datasize(const char *, const char *); extern int git_config_set(const char *, const char *); extern int git_config_set_multivar(const char *, const char *, const char *, int); extern int git_config_rename_section(const char *, const char *); diff --git a/config.c b/config.c index 2e0d5a8..07ad2f1 100644 --- a/config.c +++ b/config.c @@ -255,6 +255,31 @@ int git_config_bool(const char *name, const char *value) return git_config_int(name, value) != 0; } +size_t git_config_datasize(const char *name, const char *value) +{ + if (value && *value) { + char *end; + unsigned long val = strtoul(value, &end, 0); + while (isspace(*end)) + end++; + if (!*end) + return val; + if (!strcasecmp(end, "k") + || !strcasecmp(end, "kb") + || !strcasecmp(end, "kib")) + return val * 1024; + if (!strcasecmp(end, "m") + || !strcasecmp(end, "mb") + || !strcasecmp(end, "mib")) + return val * 1024 * 1024; + if (!strcasecmp(end, "g") + || !strcasecmp(end, "gb") + || !strcasecmp(end, "gib")) + return val * 1024 * 1024 * 1024; + } + die("bad config value for '%s' in %s", name, config_file_name); +} + int git_default_config(const char *var, const char *value) { /* This needs a better name */ -- 1.5.0.rc0.g6bb1 ``` ## Junio C Hamano, 2006-12-31 02:33 Subject: Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. Message-ID: <7vodpkhjzq.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vodpkhjzq.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20061231020218.GA5366@spearce.org> ``` "Shawn O. Pearce" writes: > Sometimes its necessary to supply a value as a power of two in a > configuration parameter. In this case the user may want to use > the standard suffixes such as K, KB, KiB, etc. to indicate that > the numerical value should be multiplied by a constant base before > being used. If you are really going to do this, I think we would need something similar to --bool to repo-config command. Also can we fix the definition of core.packedGitWindowSize to be independent of the page size on a particular platform? ``` ## Shawn Pearce, 2006-12-31 02:38 Subject: Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. Message-ID: <20061231023809.GC5082@spearce.org> URL: https://gitlist.dev/e/20061231023809.GC5082%40spearce.org In-Reply-To: <7vodpkhjzq.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > "Shawn O. Pearce" writes: > > > Sometimes its necessary to supply a value as a power of two in a > > configuration parameter. In this case the user may want to use > > the standard suffixes such as K, KB, KiB, etc. to indicate that > > the numerical value should be multiplied by a constant base before > > being used. > > If you are really going to do this, I think we would need > something similar to --bool to repo-config command. That shouldn't be difficult. > Also can we fix the definition of core.packedGitWindowSize to be > independent of the page size on a particular platform? What do you mean? mmap() only works in page units, and because of the way the code is built the minimum size we can allow is 2 pages. Asking mmap() to map less than a full page in the last page of a given window is silly, as that is just going to waste virtual address space or cause pain for the OS, depending on how that gets implemented. -- Shawn. ``` ## Junio C Hamano, 2006-12-31 02:57 Subject: Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. Message-ID: <7vk608hiwq.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vk608hiwq.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20061231023809.GC5082@spearce.org> ``` Shawn Pearce writes: >> Also can we fix the definition of core.packedGitWindowSize to be >> independent of the page size on a particular platform? > > What do you mean? mmap() only works in page units, and because of > the way the code is built the minimum size we can allow is 2 pages. > > Asking mmap() to map less than a full page in the last page of > a given window is silly, as that is just going to waste virtual > address space or cause pain for the OS, depending on how that > gets implemented. Ah, I misread the code. I thought the value is integer that states number of pages, but you are only rounding it down and enforcing two-page minimum. Sorry for the noise. ``` ## Shawn Pearce, 2006-12-31 03:01 Subject: Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. Message-ID: <20061231030119.GD5082@spearce.org> URL: https://gitlist.dev/e/20061231030119.GD5082%40spearce.org In-Reply-To: <7vk608hiwq.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Shawn Pearce writes: > > >> Also can we fix the definition of core.packedGitWindowSize to be > >> independent of the page size on a particular platform? > > > > What do you mean? mmap() only works in page units, and because of > > the way the code is built the minimum size we can allow is 2 pages. > > > > Asking mmap() to map less than a full page in the last page of > > a given window is silly, as that is just going to waste virtual > > address space or cause pain for the OS, depending on how that > > gets implemented. > > Ah, I misread the code. I thought the value is integer that > states number of pages, but you are only rounding it down and > enforcing two-page minimum. So in your test case where you thought you entered 20 pages, what you really had was just 2 pages. We rounded 20 bytes down to 0 pages, then forced that to a minimum of 2 pages. :-) -- Shawn. ``` ## Junio C Hamano, 2006-12-31 05:38 Subject: Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. Message-ID: <7vejqgk4l1.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vejqgk4l1.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20061231030119.GD5082@spearce.org> ``` Shawn Pearce writes: > So in your test case where you thought you entered 20 pages, what you > really had was just 2 pages. We rounded 20 bytes down to 0 pages, > then forced that to a minimum of 2 pages. :-) Yes. And I found something else that is interesting. I still had 2 pages windows but reduced packedgitlimit to 256kB and re-run the same test to reproduce all merges in git.git. It still did not fail, but it took even shorter wallclock time (again, it is only 3 seconds or so out of 5 1/2 minuts, so it is probably statistically insignificant). ``` ## Shawn O. Pearce, 2006-12-31 05:47 Subject: Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes. Message-ID: <20061231054741.GA6106@spearce.org> URL: https://gitlist.dev/e/20061231054741.GA6106%40spearce.org In-Reply-To: <7vejqgk4l1.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Shawn Pearce writes: > > > So in your test case where you thought you entered 20 pages, what you > > really had was just 2 pages. We rounded 20 bytes down to 0 pages, > > then forced that to a minimum of 2 pages. :-) > > Yes. > > And I found something else that is interesting. I still had 2 > pages windows but reduced packedgitlimit to 256kB and re-run the > same test to reproduce all merges in git.git. It still did not > fail, but it took even shorter wallclock time (again, it is only > 3 seconds or so out of 5 1/2 minuts, so it is probably > statistically insignificant). Its possible the packed_git.windows list is hurting us here. Its O(n) to locate a given window. The larger the ratio between packedGitWindowSize and packedGitLimit the longer that list can get, and the longer it can take to locate a window. Maybe that should be a simple red-black tree? More testing probably needs to be done though. -- Shawn. ```