Re: [PATCH 5/5] oidtree: a crit-bit tree for...

Re: [PATCH 5/5] oidtree: a crit-bit tree for odb_loose_cache

From: Junio C Hamano <hidden>
Date: 2021-06-29 14:42:10

Eric Wong [off-list ref] writes:

This saves 8K per `struct object_directory', meaning it saves
around 800MB in my case involving 100K alternates (half or more
of those alternates are unlikely to hold loose objects).

This is implemented in two parts: a generic, allocation-free
`cbtree' and the `oidtree' wrapper on top of it.  The latter
provides allocation using alloc_state as a memory pool to
improve locality and reduce free(3) overhead.

This seems to break CI test, with "fatal: not a hexadecimal oid",
perhaps because there is hardcoded 40 here?

quoted hunk

index 0000000000..bb4229210c

--- /dev/null
+++ b/t/t0069-oidtree.sh

@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='basic tests for the oidtree implementation'
+. ./test-lib.sh
+
+echoid () {
+	prefix="${1:+$1 }"
+	shift
+	while test $# -gt 0
+	do
+		echo "$1"
+		shift
+	done | awk -v prefix="$prefix" '{
+		printf("%s%s", prefix, $0);
+		need = 40 - length($0);
+		for (i = 0; i < need; i++)
+			printf("0");
+		printf "\n";
+	}'

Re: [PATCH 5/5] oidtree: a crit-bit tree for odb_loose_cache

From: Eric Wong <hidden>
Date: 2021-06-29 20:17:22

Junio C Hamano [off-list ref] wrote:

Eric Wong [off-list ref] writes:

quoted

This saves 8K per `struct object_directory', meaning it saves
around 800MB in my case involving 100K alternates (half or more
of those alternates are unlikely to hold loose objects).

This is implemented in two parts: a generic, allocation-free
`cbtree' and the `oidtree' wrapper on top of it.  The latter
provides allocation using alloc_state as a memory pool to
improve locality and reduce free(3) overhead.

This seems to break CI test, with "fatal: not a hexadecimal oid",
perhaps because there is hardcoded 40 here?

Yes, I think this needs to be squashed in:
--------8<------
Subject: [PATCH] t0069: make oidtree test hash-agnostic

Tested with both:

make -C t t0069-oidtree.sh GIT_TEST_DEFAULT_HASH=sha1
make -C t t0069-oidtree.sh GIT_TEST_DEFAULT_HASH=sha256

Signed-off-by: Eric Wong <redacted>
---
 t/helper/test-oidtree.c | 14 ++++++++------
 t/t0069-oidtree.sh      |  4 ++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/t/helper/test-oidtree.c b/t/helper/test-oidtree.c
index 44bb2e7c29..e0da13eea3 100644
--- a/t/helper/test-oidtree.c
+++ b/t/helper/test-oidtree.c

@@ -13,6 +13,7 @@ int cmd__oidtree(int argc, const char **argv)
 	struct oidtree ot = OIDTREE_INIT;
 	struct strbuf line = STRBUF_INIT;
 	int nongit_ok;
+	int algo = GIT_HASH_UNKNOWN;
 
 	setup_git_directory_gently(&nongit_ok);

@@ -21,20 +22,21 @@ int cmd__oidtree(int argc, const char **argv)
 		struct object_id oid;
 
 		if (skip_prefix(line.buf, "insert ", &arg)) {
-			if (get_oid_hex(arg, &oid))
-				die("not a hexadecimal oid: %s", arg);
+			if (get_oid_hex_any(arg, &oid) == GIT_HASH_UNKNOWN)
+				die("insert not a hexadecimal oid: %s", arg);
+			algo = oid.algo;
 			oidtree_insert(&ot, &oid);
 		} else if (skip_prefix(line.buf, "contains ", &arg)) {
 			if (get_oid_hex(arg, &oid))
-				die("not a hexadecimal oid: %s", arg);
+				die("contains not a hexadecimal oid: %s", arg);
 			printf("%d\n", oidtree_contains(&ot, &oid));
 		} else if (skip_prefix(line.buf, "each ", &arg)) {
-			char buf[GIT_SHA1_HEXSZ  + 1] = { '0' };
+			char buf[GIT_MAX_HEXSZ + 1] = { '0' };
 			memset(&oid, 0, sizeof(oid));
 			memcpy(buf, arg, strlen(arg));
-			buf[GIT_SHA1_HEXSZ] = 0;
+			buf[hash_algos[algo].hexsz] = 0;
 			get_oid_hex_any(buf, &oid);
-			oid.algo = GIT_HASH_SHA1;
+			oid.algo = algo;
 			oidtree_each(&ot, &oid, strlen(arg), print_oid, NULL);
 		} else if (!strcmp(line.buf, "destroy"))
 			oidtree_destroy(&ot);

diff --git a/t/t0069-oidtree.sh b/t/t0069-oidtree.sh
index bb4229210c..0594f57c81 100755
--- a/t/t0069-oidtree.sh
+++ b/t/t0069-oidtree.sh

@@ -10,9 +10,9 @@ echoid () {
 	do
 		echo "$1"
 		shift
-	done | awk -v prefix="$prefix" '{
+	done | awk -v prefix="$prefix" -v ZERO_OID=$ZERO_OID '{
 		printf("%s%s", prefix, $0);
-		need = 40 - length($0);
+		need = length(ZERO_OID) - length($0);
 		for (i = 0; i < need; i++)
 			printf("0");
 		printf "\n";

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help