git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: sbeller@google.com
Cc: git@vger.kernel.org, gitster@pobox.com, jamill@microsoft.com,
	jonathantanmy@google.com, pclouds@gmail.com
Subject: [PATCH v4 00/13] object store: alloc
Date: Wed,  9 May 2018 17:40:11 -0700	[thread overview]
Message-ID: <20180510004024.93974-1-sbeller@google.com> (raw)
In-Reply-To: <20180508193736.14883-1-sbeller@google.com>

v4:
* address the memory issues, an interdiff is below.

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/20180430153122.243976-1-jamill@microsoft.com/

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           |  65 ++++++++++++++++----------
 alloc.h           |  19 ++++++++
 blame.c           |   3 +-
 blob.c            |   5 +-
 cache.h           |   9 ----
 commit.c          |  11 ++++-
 commit.h          |   6 +++
 merge-recursive.c |   3 +-
 object.c          | 113 ++++++++++++++++++++++++++++++++++------------
 object.h          |  18 +++++++-
 repository.c      |   7 +++
 repository.h      |   9 ++++
 tag.c             |   9 +++-
 tag.h             |   1 +
 tree.c            |   4 +-
 15 files changed, 214 insertions(+), 68 deletions(-)
 create mode 100644 alloc.h

-- 
2.17.0.255.g8bfb7c0704

diff --git c/alloc.c w/alloc.c
index 4ecf0f160f4..714df633169 100644
--- c/alloc.c
+++ w/alloc.c
@@ -47,6 +47,8 @@ void clear_alloc_state(struct alloc_state *s)
 		s->slab_nr--;
 		free(s->slabs[s->slab_nr]);
 	}
+
+	FREE_AND_NULL(s->slabs);
 }
 
 static inline void *alloc_node(struct alloc_state *s, size_t node_size)
@@ -110,22 +112,6 @@ void *alloc_commit_node(struct repository *r)
 	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",
diff --git c/alloc.h w/alloc.h
index 941d71960fb..3e4e828db48 100644
--- c/alloc.h
+++ w/alloc.h
@@ -16,8 +16,4 @@ unsigned int alloc_commit_index(struct repository *r);
 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 c/commit.c w/commit.c
index c3b400d5930..612ccf7b053 100644
--- c/commit.c
+++ w/commit.c
@@ -297,6 +297,13 @@ void free_commit_buffer(struct commit *commit)
 	}
 }
 
+void release_commit_memory(struct commit *c)
+{
+	free_commit_buffer(c);
+	free_commit_list(c->parents);
+	/* TODO: what about commit->util? */
+}
+
 const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
 {
 	struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
diff --git c/commit.h w/commit.h
index 0fb8271665c..2d764ab7d8e 100644
--- c/commit.h
+++ w/commit.h
@@ -99,6 +99,12 @@ void unuse_commit_buffer(const struct commit *, const void *buffer);
  */
 void free_commit_buffer(struct commit *);
 
+/*
+ * Release memory related to a commit, including the parent list and
+ * any cached object buffer.
+ */
+void release_commit_memory(struct commit *c);
+
 /*
  * Disassociate any cached object buffer from the commit, but do not free it.
  * The buffer (or NULL, if none) is returned.
diff --git c/object.c w/object.c
index 803d34ae189..9d5b10d5a20 100644
--- c/object.c
+++ w/object.c
@@ -524,11 +524,11 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
 			continue;
 
 		if (obj->type == OBJ_TREE)
-			release_tree_node((struct tree*)obj);
+			free_tree_buffer((struct tree*)obj);
 		else if (obj->type == OBJ_COMMIT)
-			release_commit_node((struct commit*)obj);
+			release_commit_memory((struct commit*)obj);
 		else if (obj->type == OBJ_TAG)
-			release_tag_node((struct tag*)obj);
+			free_tag_buffer((struct tag*)obj);
 	}
 
 	FREE_AND_NULL(o->obj_hash);
@@ -539,4 +539,9 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
 	clear_alloc_state(o->commit_state);
 	clear_alloc_state(o->tag_state);
 	clear_alloc_state(o->object_state);
+	FREE_AND_NULL(o->blob_state);
+	FREE_AND_NULL(o->tree_state);
+	FREE_AND_NULL(o->commit_state);
+	FREE_AND_NULL(o->tag_state);
+	FREE_AND_NULL(o->object_state);
 }
diff --git c/tag.c w/tag.c
index af6a0725b6a..254352c30c6 100644
--- c/tag.c
+++ w/tag.c
@@ -116,6 +116,11 @@ static timestamp_t parse_tag_date(const char *buf, const char *tail)
 	return parse_timestamp(dateptr, NULL, 10);
 }
 
+void free_tag_buffer(struct tag *t)
+{
+	free(t->tag);
+}
+
 int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)
 {
 	struct object_id oid;
diff --git c/tag.h w/tag.h
index d469534e82a..b241fe67bc5 100644
--- c/tag.h
+++ w/tag.h
@@ -15,6 +15,7 @@ struct tag {
 extern struct tag *lookup_tag(const struct object_id *oid);
 extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long size);
 extern int parse_tag(struct tag *item);
+extern void free_tag_buffer(struct tag *t);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
 extern int gpg_verify_tag(const struct object_id *oid,

  parent reply	other threads:[~2018-05-10  0:40 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-01 21:33 [PATCH 00/13] object store: alloc Stefan Beller
2018-05-01 21:33 ` [PATCH 01/13] repository: introduce object parser field Stefan Beller
2018-05-02 17:17   ` Duy Nguyen
2018-05-02 17:26     ` Stefan Beller
2018-05-02 17:58       ` Duy Nguyen
2018-05-02 20:30   ` Jonathan Tan
2018-05-01 21:33 ` [PATCH 02/13] object: add repository argument to create_object Stefan Beller
2018-05-01 21:43   ` Eric Sunshine
2018-05-01 21:33 ` [PATCH 03/13] object: add repository argument to grow_object_hash Stefan Beller
2018-05-01 21:33 ` [PATCH 04/13] alloc: add repository argument to alloc_blob_node Stefan Beller
2018-05-02 20:34   ` Jonathan Tan
2018-05-01 21:33 ` [PATCH 05/13] alloc: add repository argument to alloc_tree_node Stefan Beller
2018-05-01 21:33 ` [PATCH 06/13] alloc: add repository argument to alloc_commit_node Stefan Beller
2018-05-01 21:33 ` [PATCH 07/13] alloc: add repository argument to alloc_tag_node Stefan Beller
2018-05-01 21:33 ` [PATCH 08/13] alloc: add repository argument to alloc_object_node Stefan Beller
2018-05-01 21:33 ` [PATCH 09/13] alloc: add repository argument to alloc_report Stefan Beller
2018-05-01 21:34 ` [PATCH 10/13] alloc: add repository argument to alloc_commit_index Stefan Beller
2018-05-01 21:34 ` [PATCH 11/13] object: allow grow_object_hash to handle arbitrary repositories Stefan Beller
2018-05-01 21:34 ` [PATCH 12/13] object: allow create_object " Stefan Beller
2018-05-02 20:36   ` Jonathan Tan
2018-05-01 21:34 ` [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions Stefan Beller
2018-05-02 17:44   ` Duy Nguyen
2018-05-03 17:24     ` Stefan Beller
2018-05-03 17:35       ` Duy Nguyen
2018-05-02 20:50   ` Jonathan Tan
2018-05-03 17:25     ` Stefan Beller
2018-05-03 14:58   ` Duy Nguyen
2018-05-02 17:01 ` [PATCH 00/13] object store: alloc Duy Nguyen
2018-05-02 18:07   ` Jameson Miller
2018-05-02 18:22     ` Duy Nguyen
2018-05-02 18:44       ` Jameson Miller
2018-05-03 22:45       ` Stefan Beller
2018-05-07 14:05 ` Junio C Hamano
2018-05-07 20:53   ` Stefan Beller
2018-05-07 22:59 ` [PATCH v2 " Stefan Beller
2018-05-07 22:59   ` [PATCH v2 01/13] repository: introduce parsed objects field Stefan Beller
2018-05-08 17:23     ` Jonathan Tan
2018-05-07 22:59   ` [PATCH v2 02/13] object: add repository argument to create_object Stefan Beller
2018-05-07 22:59   ` [PATCH v2 03/13] object: add repository argument to grow_object_hash Stefan Beller
2018-05-07 22:59   ` [PATCH v2 04/13] alloc: add repository argument to alloc_blob_node Stefan Beller
2018-05-07 22:59   ` [PATCH v2 05/13] alloc: add repository argument to alloc_tree_node Stefan Beller
2018-05-07 22:59   ` [PATCH v2 06/13] alloc: add repository argument to alloc_commit_node Stefan Beller
2018-05-07 22:59   ` [PATCH v2 07/13] alloc: add repository argument to alloc_tag_node Stefan Beller
2018-05-07 22:59   ` [PATCH v2 08/13] alloc: add repository argument to alloc_object_node Stefan Beller
2018-05-07 22:59   ` [PATCH v2 09/13] alloc: add repository argument to alloc_report Stefan Beller
2018-05-07 22:59   ` [PATCH v2 10/13] alloc: add repository argument to alloc_commit_index Stefan Beller
2018-05-07 22:59   ` [PATCH v2 11/13] object: allow grow_object_hash to handle arbitrary repositories Stefan Beller
2018-05-07 22:59   ` [PATCH v2 12/13] object: allow create_object " Stefan Beller
2018-05-07 22:59   ` [PATCH v2 13/13] alloc: allow arbitrary repositories for alloc functions Stefan Beller
2018-05-08 10:10     ` Jeff King
2018-05-08 15:00     ` Duy Nguyen
2018-05-08 18:38       ` Stefan Beller
2018-05-08 17:45     ` Jonathan Tan
2018-05-08 19:37   ` [PATCH v3 00/13] object store: alloc Stefan Beller
2018-05-08 19:37     ` [PATCH v3 01/13] repository: introduce parsed objects field Stefan Beller
2018-05-08 19:37     ` [PATCH v3 02/13] object: add repository argument to create_object Stefan Beller
2018-05-08 19:37     ` [PATCH v3 03/13] object: add repository argument to grow_object_hash Stefan Beller
2018-05-08 19:37     ` [PATCH v3 04/13] alloc: add repository argument to alloc_blob_node Stefan Beller
2018-05-08 19:37     ` [PATCH v3 05/13] alloc: add repository argument to alloc_tree_node Stefan Beller
2018-05-08 19:37     ` [PATCH v3 06/13] alloc: add repository argument to alloc_commit_node Stefan Beller
2018-05-08 19:37     ` [PATCH v3 07/13] alloc: add repository argument to alloc_tag_node Stefan Beller
2018-05-08 19:37     ` [PATCH v3 08/13] alloc: add repository argument to alloc_object_node Stefan Beller
2018-05-08 19:37     ` [PATCH v3 09/13] alloc: add repository argument to alloc_report Stefan Beller
2018-05-08 19:37     ` [PATCH v3 10/13] alloc: add repository argument to alloc_commit_index Stefan Beller
2018-05-08 19:37     ` [PATCH v3 11/13] object: allow grow_object_hash to handle arbitrary repositories Stefan Beller
2018-05-08 19:37     ` [PATCH v3 12/13] object: allow create_object " Stefan Beller
2018-05-08 19:37     ` [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions Stefan Beller
2018-05-08 20:04       ` Jonathan Tan
2018-05-08 20:37         ` Stefan Beller
2018-05-09 15:54           ` Duy Nguyen
2018-05-09 17:18         ` Duy Nguyen
2018-05-09 19:20           ` Stefan Beller
2018-05-10 15:43             ` Duy Nguyen
2018-05-10  0:40     ` Stefan Beller [this message]
2018-05-10  0:40       ` [PATCH v4 01/13] repository: introduce parsed objects field Stefan Beller
2018-05-10  0:40       ` [PATCH v4 02/13] object: add repository argument to create_object Stefan Beller
2018-05-10  0:40       ` [PATCH v4 03/13] object: add repository argument to grow_object_hash Stefan Beller
2018-05-10  0:40       ` [PATCH v4 04/13] alloc: add repository argument to alloc_blob_node Stefan Beller
2018-05-10  0:40       ` [PATCH v4 05/13] alloc: add repository argument to alloc_tree_node Stefan Beller
2018-05-10  0:40       ` [PATCH v4 06/13] alloc: add repository argument to alloc_commit_node Stefan Beller
2018-05-10  0:40       ` [PATCH v4 07/13] alloc: add repository argument to alloc_tag_node Stefan Beller
2018-05-10  0:40       ` [PATCH v4 08/13] alloc: add repository argument to alloc_object_node Stefan Beller
2018-05-10  0:40       ` [PATCH v4 09/13] alloc: add repository argument to alloc_report Stefan Beller
2018-05-10  0:40       ` [PATCH v4 10/13] alloc: add repository argument to alloc_commit_index Stefan Beller
2018-05-10  0:40       ` [PATCH v4 11/13] object: allow grow_object_hash to handle arbitrary repositories Stefan Beller
2018-05-10  0:40       ` [PATCH v4 12/13] object: allow create_object " Stefan Beller
2018-05-10  0:40       ` [PATCH v4 13/13] alloc: allow arbitrary repositories for alloc functions Stefan Beller
2018-05-10 17:16       ` [PATCH v4 00/13] object store: alloc Jonathan Tan
2018-05-10 17:32         ` Stefan Beller
2018-05-10 20:56           ` Jonathan Tan
2018-05-10 22:36             ` Stefan Beller
2018-05-11 19:17         ` [PATCH] alloc: allow arbitrary repositories for alloc functions Stefan Beller
2018-05-11 19:38           ` Eric Sunshine
2018-05-15 21:48             ` Stefan Beller
2018-05-16  2:27               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180510004024.93974-1-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jamill@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).