threads / patch / 6162

patch, 2 partsTeach Git how to parse standard power of 2 suffixes.

Subject: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.

## tl;dr

7 messages between Dec 31, 2006 and Dec 31, 2006. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Shawn O. Pearce· Dec 31, 2006, 02:02 UTC · lore

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 <spearce@spearce.org>
---
 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(-)
Show changes to 2 files +26 −0

cache.h, config.c

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· Dec 31, 2006, 02:33 UTC · re: Shawn O. Pearce · lore

Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 5 quoted lines
> 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· Dec 31, 2006, 02:38 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.

Junio C Hamano <junkio@cox.net> wrote:
Show 10 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> 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· Dec 31, 2006, 02:57 UTC · re: Shawn Pearce · lore

Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.

Shawn Pearce <spearce@spearce.org> writes:
Show 10 quoted 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.
Shawn Pearce· Dec 31, 2006, 03:01 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.

Junio C Hamano <junkio@cox.net> wrote:
Show 16 quoted lines
> Shawn Pearce <spearce@spearce.org> 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· Dec 31, 2006, 05:38 UTC · re: Shawn Pearce · lore

Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.

Shawn Pearce <spearce@spearce.org> 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· Dec 31, 2006, 05:47 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.

Junio C Hamano <junkio@cox.net> wrote:
Show 14 quoted lines
> Shawn Pearce <spearce@spearce.org> 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.

← back to recent threads