Re: [PATCH] fast-export: avoid NULL pointer arithmetic

[Duy Nguyen <pclouds@gmail.com>]

On Fri, May 11, 2018 at 03:11:46PM +0200, Duy Nguyen wrote:
> Back to fast-export, can we just allocate a new int on heap and point
> it there? Allocating small pieces becomes quite cheap and fast with
> mem-pool.h and we can avoid this storing integer in pointer business.

Something like this seems to work, but we use 4-ish more bytes per
object, or 100MB overhead on a repo with 25M objects. I think it's a
reasonable trade off.

-- 8< --
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 530df12f05..de593035b1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -21,6 +21,7 @@
 #include "quote.h"
 #include "remote.h"
 #include "blob.h"
+#include "mem-pool.h"
 
 static const char *fast_export_usage[] = {
 	N_("git fast-export [rev-list-opts]"),
@@ -38,6 +39,7 @@ static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
 static struct refspec *refspecs;
 static int refspecs_nr;
 static int anonymize;
+static struct mem_pool int_pool = MEM_POOL_INIT(2 * 1024 * 1024);
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -156,20 +158,22 @@ static void anonymize_path(struct strbuf *out, const char *path,
 	}
 }
 
-/* Since intptr_t is C99, we do not use it here */
-static inline uint32_t *mark_to_ptr(uint32_t mark)
+static inline uint32_t ptr_to_mark(void *mark)
 {
-	return ((uint32_t *)NULL) + mark;
-}
-
-static inline uint32_t ptr_to_mark(void * mark)
-{
-	return (uint32_t *)mark - (uint32_t *)NULL;
+	if (!mark)
+		BUG("not marked!");
+	return *(uint32_t *)mark;
 }
 
 static inline void mark_object(struct object *object, uint32_t mark)
 {
-	add_decoration(&idnums, object, mark_to_ptr(mark));
+	uint32_t *ptr = lookup_decoration(&idnums, object);
+
+	if (!ptr)
+		ptr = mem_pool_alloc(&int_pool, sizeof(uint32_t));
+
+	*ptr = mark;
+	add_decoration(&idnums, object, ptr);
 }
 
 static inline void mark_next_object(struct object *object)
diff --git a/fast-import.c b/fast-import.c
index 34edf3fb8f..ce5ce2081f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -300,8 +300,7 @@ static int global_argc;
 static const char **global_argv;
 
 /* Memory pools */
-static struct mem_pool fi_mem_pool =  {NULL, 2*1024*1024 -
-				       sizeof(struct mp_block), 0 };
+static struct mem_pool fi_mem_pool = MEM_POOL_INIT(2*1024*1024);
 
 /* Atom management */
 static unsigned int atom_table_sz = 4451;
diff --git a/mem-pool.h b/mem-pool.h
index 829ad58ecf..bccbd3f224 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -21,6 +21,8 @@ struct mem_pool {
 	size_t pool_alloc;
 };
 
+#define MEM_POOL_INIT(block_size) { NULL, (block_size) - sizeof(struct mp_block), 0 }
+
 /*
  * Alloc memory from the mem_pool.
  */
-- 8< --