Teach Git how to parse standard power of 2 suffixes.
PATCH 1/2 Teach Git how to parse standard power of 2 suffixes.
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.
---
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(-)
cache.h +1 -0
@@ -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 *);
config.c +25 -0
@@ -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 */
Signature · 1 line
1.5.0.rc0.g6bb1
Quoted 5 lines · Shawn O. Pearce
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?
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Shawn O. Pearce
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.
Quoted 2 lines
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.
Signature · 1 line
Shawn.
Quoted 10 lines · Shawn Pearce
Quoted 2 lines
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.
Quoted 8 lines · Junio C Hamano
Quoted 6 lines · Shawn Pearce
Quoted 2 lines
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.
Quoted 8 lines
Quoted 4 lines
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. :-)
Signature · 1 line
Shawn.
Quoted 3 lines · Shawn Pearce
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).
Quoted 6 lines · Junio C Hamano
Quoted 3 lines · Shawn Pearce
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. :-)
Quoted 8 lines
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.
Signature · 1 line
Shawn.