Subject: [PATCH v3 00/13] object store: alloc

From: Stefan Beller <[email-masked]>
Date: Tue, 8 May 2018 12:37:23 -0700

v3:

* I used the (soon to be renamed?) branch-diff tool to attach a diff below
  between v2 and v3 
  
* fixed comment in patch 1
* correctly free objects and its hashmap in the last patch.
* drop free'ing the commit->util pointer as we do not know where
  it points to.

v2:
* I decided to stick with alloc.c and not migrate it to the mem-pool for now.
  The reasoning for that is that mem-pool.h would introduce some alignment
  waste, which I really did not want to.
* renamed to struct parsed_object_pool;
* free'd the additional memory of trees and commits.
* do not special case the_repository for allocation purposes
* corrected&polished commit messages
* I used the (soon to be renamed?) branch-diff tool to attach a diff below.
  (I still need to get used to that format, I find an interdiff of the
   branches easier to read, but that would not yield the commit messages)

v1:
This applies on top of sb/oid-object-info and is the logical continuum of
the series that it builds on; this brings the object store into more of
Gits code, removing global state, such that reasoning about the state of
the in-memory representation of the repository is easier.

My original plan was to convert lookup_commit_graft as the next series,
which would be similar to lookup_replace_object, as in sb/object-store-replace.
The grafts and shallow mechanism are very close to each other, such that
they need to be converted at the same time, both depending on the
"parsed object store" that is introduced in this commit.

The next series will then convert code in {object/blob/tree/commit/tag}.c
hopefully finishing the lookup_* functions.

I also debated if it is worth converting alloc.c via this patch series
or if it might make more sense to use the new mem-pool by Jameson[1].

I vaguely wonder about the performance impact, as the object allocation
code seemed to be relevant in the past.

[1] https://public-inbox.org/git/[email-masked]/

Any comments welcome,
Thanks,
Stefan

Jonathan Nieder (1):
  object: add repository argument to grow_object_hash

Stefan Beller (12):
  repository: introduce parsed objects field
  object: add repository argument to create_object
  alloc: add repository argument to alloc_blob_node
  alloc: add repository argument to alloc_tree_node
  alloc: add repository argument to alloc_commit_node
  alloc: add repository argument to alloc_tag_node
  alloc: add repository argument to alloc_object_node
  alloc: add repository argument to alloc_report
  alloc: add repository argument to alloc_commit_index
  object: allow grow_object_hash to handle arbitrary repositories
  object: allow create_object to handle arbitrary repositories
  alloc: allow arbitrary repositories for alloc functions

 alloc.c           |  79 ++++++++++++++++++++++-----------
 alloc.h           |  23 ++++++++++
 blame.c           |   3 +-
 blob.c            |   5 ++-
 cache.h           |   9 ----
 commit.c          |   4 +-
 merge-recursive.c |   3 +-
 object.c          | 108 ++++++++++++++++++++++++++++++++++------------
 object.h          |  18 +++++++-
 repository.c      |   7 +++
 repository.h      |   9 ++++
 tag.c             |   4 +-
 tree.c            |   4 +-
 13 files changed, 208 insertions(+), 68 deletions(-)
 create mode 100644 alloc.h

-- 
2.17.0.255.g8bfb7c0704

1:  9efc685875b ! 1:  f8e521c7c11 repository: introduce parsed objects field
    @@ -15,7 +15,6 @@
         discussed on the mailing list lately, this series doesn't implement this.
     
         Signed-off-by: Stefan Beller <[email-masked]>
    -    Signed-off-by: Junio C Hamano <[email-masked]>
     
     diff --git a/object.c b/object.c
     --- a/object.c
    @@ -224,13 +223,6 @@
     --- a/repository.h
     +++ b/repository.h
     @@
    - 	char *commondir;
    - 
    - 	/*
    --	 * Holds any information related to accessing the raw object content.
    -+	 * Holds any information needed to retrieve the raw content
    -+	 * of objects. The object_parser uses this to get object
    -+	 * content which it then parses.
      	 */
      	struct raw_object_store *objects;
      
2:  0d41290a9e6 = 2:  55c555b32eb object: add repository argument to create_object
3:  0242ec870f5 = 3:  f1661c9e46a object: add repository argument to grow_object_hash
4:  9a6aeee10db = 4:  f72b25db946 alloc: add repository argument to alloc_blob_node
5:  f7ed8da3909 = 5:  87b7ddda195 alloc: add repository argument to alloc_tree_node
6:  253f1bf5c2a = 6:  4480e916bdf alloc: add repository argument to alloc_commit_node
7:  4f8d3dfd460 = 7:  c3aa2a7c252 alloc: add repository argument to alloc_tag_node
8:  6ce5d5b0f0e = 8:  59d33cfaff2 alloc: add repository argument to alloc_object_node
9:  104f158fc37 = 9:  2ba78c289c1 alloc: add repository argument to alloc_report
10:  38d90052c29 = 10:  10ce6c44d4b alloc: add repository argument to alloc_commit_index
11:  eae3dea5763 = 11:  eae95e75b0b object: allow grow_object_hash to handle arbitrary repositories
12:  d08b382662f = 12:  c6d86c8b5db object: allow create_object to handle arbitrary repositories
13:  f87e600c439 ! 13:  2a56520e7af alloc: allow arbitrary repositories for alloc functions
    @@ -139,6 +139,25 @@
      	return c;
      }
      
    ++void release_tree_node(struct tree *t)
    ++{
    ++	free(t->buffer);
    ++}
    ++
    ++void release_commit_node(struct commit *c)
    ++{
    ++	free_commit_list(c->parents);
    ++	/* TODO: what about commit->util? */
    ++}
    ++
    ++void release_tag_node(struct tag *t)
    ++{
    ++	free(t->tag);
    ++}
    ++
    + static void report(const char *name, unsigned int count, size_t size)
    + {
    + 	fprintf(stderr, "%10s: %8u (%"PRIuMAX" kB)\n",
     @@
      }
      
    @@ -161,6 +180,10 @@
     +#ifndef ALLOC_H
     +#define ALLOC_H
     +
    ++struct tree;
    ++struct commit;
    ++struct tag;
    ++
     +void *alloc_blob_node(struct repository *r);
     +void *alloc_tree_node(struct repository *r);
     +void *alloc_commit_node(struct repository *r);
    @@ -172,6 +195,10 @@
     +void *allocate_alloc_state(void);
     +void clear_alloc_state(struct alloc_state *s);
     +
    ++void release_tree_node(struct tree *t);
    ++void release_commit_node(struct commit *c);
    ++void release_tag_node(struct tag *t);
    ++
     +#endif
     
     diff --git a/blame.c b/blame.c
    @@ -241,25 +268,25 @@
     --- a/merge-recursive.c
     +++ b/merge-recursive.c
     @@
    - #include "tree-walk.h"
      #include "diff.h"
      #include "diffcore.h"
    -+#include "alloc.h"
      #include "tag.h"
    ++#include "alloc.h"
      #include "unpack-trees.h"
      #include "string-list.h"
    + #include "xdiff-interface.h"
     
     diff --git a/object.c b/object.c
     --- a/object.c
     +++ b/object.c
     @@
    - #include "blob.h"
      #include "tree.h"
      #include "commit.h"
    -+#include "alloc.h"
      #include "tag.h"
    ++#include "alloc.h"
      #include "object-store.h"
      #include "packfile.h"
    + 
     @@
      {
      	struct parsed_object_pool *o = xmalloc(sizeof(*o));
    @@ -294,14 +321,17 @@
     +		if (!obj)
     +			continue;
     +
    -+		if (obj->type == OBJ_TREE) {
    -+			free(((struct tree*)obj)->buffer);
    -+		} else if (obj->type == OBJ_COMMIT) {
    -+			free_commit_list(((struct commit*)obj)->parents);
    -+			free(&((struct commit*)obj)->util);
    -+		}
    ++		if (obj->type == OBJ_TREE)
    ++			release_tree_node((struct tree*)obj);
    ++		else if (obj->type == OBJ_COMMIT)
    ++			release_commit_node((struct commit*)obj);
    ++		else if (obj->type == OBJ_TAG)
    ++			release_tag_node((struct tag*)obj);
     +	}
     +
    ++	FREE_AND_NULL(o->obj_hash);
    ++	o->obj_hash_size = 0;
    ++
     +	clear_alloc_state(o->blob_state);
     +	clear_alloc_state(o->tree_state);
     +	clear_alloc_state(o->commit_state);