diff mbox series

[v2] alloc.h|c: migrate alloc_states to mem-pool

Message ID pull.857.v2.git.1612175966786.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v2] alloc.h|c: migrate alloc_states to mem-pool | expand

Commit Message

ZheNing Hu Feb. 1, 2021, 10:39 a.m. UTC
From: ZheNing Hu <adlternative@gmail.com>

"alloc_state" may have similar effects with "mem_pool".
Using the new memory pool API may be more beneficial
to our memory management in the future.

So I change them in the "struct parsed_object_pool",and
The corresponding interface has also been changed.
functions "alloc_*_node" now change to "mem_pool_alloc_*_node".

At the same time ,I add the member `alloc_count` of
struct mem_pool ,so that we can effective track
node alloc count,and adapt to the original interface `alloc_report`.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
    object.h: migrate alloc_states to mem-pool
    
    Notice that "mem-pool" api may have similar effort with alloc_state,
    "parsed_object_pool" have five member with alloc_state type, and "TODO"
    usage in "object.h":"migrate alloc_states to mem-pool?", so let us
    change it to mem-pool version.
    
    After I learned the role of the memory pool,I think in the future git
    may be more inclined to use the memory pool instead of the old interface
    "alloc_state".
    
    Thanks.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-857%2Fadlternative%2Falloc_states_to_mem_pool-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-857/adlternative/alloc_states_to_mem_pool-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/857

Range-diff vs v1:

 1:  3adcd229be7 ! 1:  e9c1f9eef42 alloc.h|c: migrate alloc_states to mem-pool
     @@ mem-pool.h: struct mem_pool {
        * Initialize mem_pool with specified initial size.
        */
      
     + ## merge-ort.c ##
     +@@ merge-ort.c: static struct commit *make_virtual_commit(struct repository *repo,
     + 					  struct tree *tree,
     + 					  const char *comment)
     + {
     +-	struct commit *commit = alloc_commit_node(repo);
     ++	struct commit *commit = mem_pool_alloc_commit_node(repo);
     + 
     + 	set_merge_remote_desc(commit, comment, (struct object *)commit);
     + 	set_commit_tree(commit, tree);
     +
       ## merge-recursive.c ##
      @@ merge-recursive.c: static struct commit *make_virtual_commit(struct repository *repo,
       					  struct tree *tree,


 alloc.c           | 24 ++++++++++++------------
 alloc.h           | 10 +++++-----
 blame.c           |  2 +-
 blob.c            |  2 +-
 commit-graph.c    |  2 +-
 commit.c          |  2 +-
 mem-pool.c        |  6 ++++++
 mem-pool.h        |  8 ++++++++
 merge-ort.c       |  2 +-
 merge-recursive.c |  2 +-
 object.c          | 37 +++++++++++++++++++++----------------
 object.h          | 10 +++++-----
 tag.c             |  2 +-
 tree.c            |  2 +-
 14 files changed, 65 insertions(+), 46 deletions(-)


base-commit: e6362826a0409539642a5738db61827e5978e2e4

Comments

René Scharfe. Feb. 1, 2021, 5:55 p.m. UTC | #1
Am 01.02.21 um 11:39 schrieb 阿德烈 via GitGitGadget:
> From: ZheNing Hu <adlternative@gmail.com>
>
> "alloc_state" may have similar effects with "mem_pool".
> Using the new memory pool API may be more beneficial
> to our memory management in the future.

Replacing the custom object allocator with mem-pool would allow reducing
the code size.  What other effects might it have?  Do you expect changes
in memory use and/or performance with the current code and your patch?

> functions "alloc_*_node" now change to "mem_pool_alloc_*_node".

Why rename these functions?  Do callers need to care about the
underlying allocator?  The function signatures stay the same.  In any
case, this renaming would be easier to review if it was moved to a
separate patch.

> At the same time ,I add the member `alloc_count` of
> struct mem_pool ,so that we can effective track
> node alloc count,and adapt to the original interface `alloc_report`.

This function has no callers.  Why not remove it (in a separate patch)?

> diff --git a/alloc.c b/alloc.c
> index 957a0af3626..951ef3e4ed7 100644
> --- a/alloc.c
> +++ b/alloc.c
> @@ -71,30 +71,30 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)
>  	return ret;
>  }

This keeps the now unused function alloc_node(), which breaks the build
with -Werror.

allocate_alloc_state() and clear_alloc_state() become unused as well,
but the compiler doesn't complain because those functions are
exported.  Nevertheless this patch should remove them, no?

> diff --git a/object.h b/object.h
> index 59daadce214..43031d8dc04 100644
> --- a/object.h
> +++ b/object.h
> @@ -10,11 +10,11 @@ struct parsed_object_pool {
>  	int nr_objs, obj_hash_size;
>
>  	/* TODO: migrate alloc_states to mem-pool? */

This comment becomes stale with this patch and should be removed at
the same time.

> -	struct alloc_state *blob_state;
> -	struct alloc_state *tree_state;
> -	struct alloc_state *commit_state;
> -	struct alloc_state *tag_state;
> -	struct alloc_state *object_state;
> +	struct mem_pool *blob_pool;
> +	struct mem_pool *tree_pool;
> +	struct mem_pool *commit_pool;
> +	struct mem_pool *tag_pool;
> +	struct mem_pool *object_pool;

Why have pointers here instead of the structs themselves?  It's not like
a struct parsed_object_pool is of much use without them, right?

The same question applies to the original code as well, of course.

René
Junio C Hamano Feb. 1, 2021, 5:56 p.m. UTC | #2
"阿德烈 via GitGitGadget"  <gitgitgadget@gmail.com> writes:

> From: ZheNing Hu <adlternative@gmail.com>
>
> "alloc_state" may have similar effects with "mem_pool".

What "similar effects" do you have in mind?  "mem_pool" may have
more than one "effects" to multiple things that are affected, but it
is unclear which effect that "mem_pool" exerts on what you are
referring to.

> Using the new memory pool API may be more beneficial
> to our memory management in the future.

Many things may or may not be "beneficial" in the future.  We do not
build things on a vague "hunch".

Are you seeking performance (e.g.  number of objects that can be
allocated per minute)?  Are you seeking better memory locality
(e.g. related objects are likely to be stored in the same page,
reducing number of page faults)?  Are you seeking reduced wasted
memory (e.g. custom allocator packs objects better than bog-standard
malloc(3))?  Are you seeking functionality (e.g. you have this and
that specific codepaths and usecase where you wish to be able to
release all the objects instantiated for a particular repository,
without having to go through the list of all objects, and use of
mempool is one way to allow us do so)?

It is not even clear in your problem description what kind of
benefit you are seeking, let alone how much quantitative improvement
you are getting with this change.
ZheNing Hu Feb. 2, 2021, 1:06 p.m. UTC | #3
To Junio:
Thanks for checking.forget my unprofessional
description.Macroscopically speaking,
both alloc_state and mem_pool are doing one thing:Apply for a
large block of memory in advance,and when needed a dynamically
allocated memory ,we call the interface function to apply for memory,
This can reduce the overhead of calling malloc multiple times.And the
mem-pool or alloc_state will Automatic Expand capacity.

So that ,my this patch may have something not considered,
>     mem_pool_init(o->blob_pool,0);
may be a wrong way to init this mem-pool
because:
>void mem_pool_init(...)
>       ...
>       if (initial_size > 0)
>              mem_pool_alloc_block(pool, initial_size, NULL);
the first time calloc malloc may  decay to we first call "mem_pool_alloc_block",
I think this may not be great.

A little ashamed,I did not consider the optimization point of this
at the beginning,


Junio C Hamano <gitster@pobox.com> 于2021年2月2日周二 上午1:56写道:
>
> "阿德烈 via GitGitGadget"  <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > "alloc_state" may have similar effects with "mem_pool".
>
> What "similar effects" do you have in mind?  "mem_pool" may have
> more than one "effects" to multiple things that are affected, but it
> is unclear which effect that "mem_pool" exerts on what you are
> referring to.
>
> > Using the new memory pool API may be more beneficial
> > to our memory management in the future.
>
> Many things may or may not be "beneficial" in the future.  We do not
> build things on a vague "hunch".
>
Now,I make a rough comparison.
Situation : when we just use it to malloc little struct node
such as `object`,`blob` ,`tree`.
> Are you seeking performance (e.g.  number of objects that can be
> allocated per minute)?

1.  performance.
`mem_pool` api will allocate 2^20 byte everytime ,
`alloc_state` api will allocate 1024*nodesize byte and ALLOC_GROW everytime.
A repo like git may call malloc fewer times when using `mem_pool`,
while a small repo may not have this amount of objects. The number of
calling `malloc`
may be similar.
may be `mem_pool` win a little...
>Are you seeking better memory locality
> (e.g. related objects are likely to be stored in the same page,
> reducing number of page faults)?
2. page faults .
I might think they are similar at first.But now,I start to understand
what you mean:`alloc_state` more like an object pool,so that we could
go through the list of all objects.Therefore, mem_pool is not conducive
to continuous access to all objects.Because There may be fragments
in the memory And this must be a cross-page operation.
so `alloc_state` win.
>Are you seeking reduced wasted
> memory (e.g. custom allocator packs objects better than bog-standard
> malloc(3))?
3.Memory utilization.
`alloc_state`win.No doubt.
 Are you seeking functionality (e.g. you have this and
> that specific codepaths and usecase where you wish to be able to
> release all the objects instantiated for a particular repository,
> without having to go through the list of all objects, and use of
> mempool is one way to allow us do so)?
>
4.functionality
yeah,As mentioned above.Object pool will be better.
`alloc_state`win.
5.
Indeed, the object pool `alloc_state` may be better than the
memory pool `mem_pool`.
But We can assume that the original author’s intention may be to
The five alloc_states are merged together.
Because the original author said: "migrate alloc_states to mem-pool"
Or another advantage of using the memory pool is that it can dynamically
allocate a variety of different objects, I now think the original author has
this intention.So my patch code also needs some modifications.But at the
same time, it may not be good to count them separately if multiple objects
are allocated using the memory pool at the same time.
so 'mem_pool' win a little.
> It is not even clear in your problem description what kind of
> benefit you are seeking, let alone how much quantitative improvement
> you are getting with this change.
>
I don't know how to quantify them temporarily.
I may need the opinions of you and the original author before I can move on.

Thanks.
ZheNing Hu Feb. 2, 2021, 1:12 p.m. UTC | #4
To René Scharfe:
Thanks for checking in.

René Scharfe <l.s.r@web.de> 于2021年2月2日周二 上午1:55写道:
>
> Am 01.02.21 um 11:39 schrieb 阿德烈 via GitGitGadget:
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > "alloc_state" may have similar effects with "mem_pool".
> > Using the new memory pool API may be more beneficial
> > to our memory management in the future.
>
> Replacing the custom object allocator with mem-pool would allow reducing
> the code size.  What other effects might it have?  Do you expect changes
> in memory use and/or performance with the current code and your patch?
>
> > functions "alloc_*_node" now change to "mem_pool_alloc_*_node".
>
> Why rename these functions?  Do callers need to care about the
> underlying allocator?  The function signatures stay the same.  In any
> case, this renaming would be easier to review if it was moved to a
> separate patch.
>
Truly,I will change it.
> > At the same time ,I add the member `alloc_count` of
> > struct mem_pool ,so that we can effective track
> > node alloc count,and adapt to the original interface `alloc_report`.
>
> This function has no callers.  Why not remove it (in a separate patch)?
>
Before I may have some confuse about choosing `alloc_state`or`mem_pool`,
so It has not been deleted yet.I remember that now.
> > diff --git a/alloc.c b/alloc.c
> > index 957a0af3626..951ef3e4ed7 100644
> > --- a/alloc.c
> > +++ b/alloc.c
> > @@ -71,30 +71,30 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)
> >       return ret;
> >  }
>
> This keeps the now unused function alloc_node(), which breaks the build
> with -Werror.
>
> allocate_alloc_state() and clear_alloc_state() become unused as well,
> but the compiler doesn't complain because those functions are
> exported.  Nevertheless this patch should remove them, no?
>
> > diff --git a/object.h b/object.h
> > index 59daadce214..43031d8dc04 100644
> > --- a/object.h
> > +++ b/object.h
> > @@ -10,11 +10,11 @@ struct parsed_object_pool {
> >       int nr_objs, obj_hash_size;
> >
> >       /* TODO: migrate alloc_states to mem-pool? */
>
> This comment becomes stale with this patch and should be removed at
> the same time.
>
OK.
> > -     struct alloc_state *blob_state;
> > -     struct alloc_state *tree_state;
> > -     struct alloc_state *commit_state;
> > -     struct alloc_state *tag_state;
> > -     struct alloc_state *object_state;
> > +     struct mem_pool *blob_pool;
> > +     struct mem_pool *tree_pool;
> > +     struct mem_pool *commit_pool;
> > +     struct mem_pool *tag_pool;
> > +     struct mem_pool *object_pool;
>
> Why have pointers here instead of the structs themselves?  It's not like
> a struct parsed_object_pool is of much use without them, right?
>
> The same question applies to the original code as well, of course.
Here I may have some questions: why use `struct mem_pool` instead of
using `struct mem_pool *`?
I hope you can answer my doubts, thank you!
>
> René
René Scharfe. Feb. 2, 2021, 4:36 p.m. UTC | #5
Am 02.02.21 um 14:12 schrieb 胡哲宁:
> To René Scharfe:
>>> -     struct alloc_state *blob_state;
>>> -     struct alloc_state *tree_state;
>>> -     struct alloc_state *commit_state;
>>> -     struct alloc_state *tag_state;
>>> -     struct alloc_state *object_state;
>>> +     struct mem_pool *blob_pool;
>>> +     struct mem_pool *tree_pool;
>>> +     struct mem_pool *commit_pool;
>>> +     struct mem_pool *tag_pool;
>>> +     struct mem_pool *object_pool;
>>
>> Why have pointers here instead of the structs themselves?  It's not like
>> a struct parsed_object_pool is of much use without them, right?
>>
>> The same question applies to the original code as well, of course.
> Here I may have some questions: why use `struct mem_pool` instead of
> using `struct mem_pool *`?
> I hope you can answer my doubts, thank you!

If struct parsed_object_pool contains pointers to five instances of
struct alloc_state or struct mem_pool then you have to allocate and
eventually release those instances explicitly.  Your patch introduced
mem_pool_new() for the allocation part.

If the five instances are embedded in struct parsed_object_pool then
you don't need to do that.

The indirection added by allocating explicitly and using pointers
would be beneficial if some of five instances were optional, as you
could skip their allocation and save some memory -- but you need
them all to get a usable struct parsed_object_pool.

René
diff mbox series

Patch

diff --git a/alloc.c b/alloc.c
index 957a0af3626..951ef3e4ed7 100644
--- a/alloc.c
+++ b/alloc.c
@@ -71,30 +71,30 @@  static inline void *alloc_node(struct alloc_state *s, size_t node_size)
 	return ret;
 }
 
-void *alloc_blob_node(struct repository *r)
+void *mem_pool_alloc_blob_node(struct repository *r)
 {
-	struct blob *b = alloc_node(r->parsed_objects->blob_state, sizeof(struct blob));
+	struct blob *b = mem_pool_calloc(r->parsed_objects->blob_pool, 1, sizeof(struct blob));
 	b->object.type = OBJ_BLOB;
 	return b;
 }
 
-void *alloc_tree_node(struct repository *r)
+void *mem_pool_alloc_tree_node(struct repository *r)
 {
-	struct tree *t = alloc_node(r->parsed_objects->tree_state, sizeof(struct tree));
+	struct tree *t = mem_pool_calloc(r->parsed_objects->tree_pool, 1, sizeof(struct tree));
 	t->object.type = OBJ_TREE;
 	return t;
 }
 
-void *alloc_tag_node(struct repository *r)
+void *mem_pool_alloc_tag_node(struct repository *r)
 {
-	struct tag *t = alloc_node(r->parsed_objects->tag_state, sizeof(struct tag));
+	struct tag *t = mem_pool_calloc(r->parsed_objects->tag_pool, 1, sizeof(struct tag));
 	t->object.type = OBJ_TAG;
 	return t;
 }
 
-void *alloc_object_node(struct repository *r)
+void *mem_pool_alloc_object_node(struct repository *r)
 {
-	struct object *obj = alloc_node(r->parsed_objects->object_state, sizeof(union any_object));
+	struct object *obj = mem_pool_calloc(r->parsed_objects->object_pool, 1, sizeof(union any_object));
 	obj->type = OBJ_NONE;
 	return obj;
 }
@@ -116,9 +116,9 @@  void init_commit_node(struct commit *c)
 	c->index = alloc_commit_index();
 }
 
-void *alloc_commit_node(struct repository *r)
+void *mem_pool_alloc_commit_node(struct repository *r)
 {
-	struct commit *c = alloc_node(r->parsed_objects->commit_state, sizeof(struct commit));
+	struct commit *c = mem_pool_calloc(r->parsed_objects->commit_pool, 1, sizeof(struct commit));
 	init_commit_node(c);
 	return c;
 }
@@ -130,8 +130,8 @@  static void report(const char *name, unsigned int count, size_t size)
 }
 
 #define REPORT(name, type)	\
-    report(#name, r->parsed_objects->name##_state->count, \
-		  r->parsed_objects->name##_state->count * sizeof(type) >> 10)
+    report(#name, r->parsed_objects->name##_pool->alloc_count, \
+		  r->parsed_objects->name##_pool->alloc_count * sizeof(type) >> 10)
 
 void alloc_report(struct repository *r)
 {
diff --git a/alloc.h b/alloc.h
index 371d388b552..707b28b464e 100644
--- a/alloc.h
+++ b/alloc.h
@@ -7,12 +7,12 @@  struct commit;
 struct tag;
 struct repository;
 
-void *alloc_blob_node(struct repository *r);
-void *alloc_tree_node(struct repository *r);
+void *mem_pool_alloc_blob_node(struct repository *r);
+void *mem_pool_alloc_tree_node(struct repository *r);
 void init_commit_node(struct commit *c);
-void *alloc_commit_node(struct repository *r);
-void *alloc_tag_node(struct repository *r);
-void *alloc_object_node(struct repository *r);
+void *mem_pool_alloc_commit_node(struct repository *r);
+void *mem_pool_alloc_tag_node(struct repository *r);
+void *mem_pool_alloc_object_node(struct repository *r);
 void alloc_report(struct repository *r);
 
 struct alloc_state *allocate_alloc_state(void);
diff --git a/blame.c b/blame.c
index a5044fcfaa6..cbb9d8316c1 100644
--- a/blame.c
+++ b/blame.c
@@ -192,7 +192,7 @@  static struct commit *fake_working_tree_commit(struct repository *r,
 
 	repo_read_index(r);
 	time(&now);
-	commit = alloc_commit_node(r);
+	commit = mem_pool_alloc_commit_node(r);
 	commit->object.parsed = 1;
 	commit->date = now;
 	parent_tail = &commit->parents;
diff --git a/blob.c b/blob.c
index 182718aba9f..787c7b2b016 100644
--- a/blob.c
+++ b/blob.c
@@ -9,7 +9,7 @@  struct blob *lookup_blob(struct repository *r, const struct object_id *oid)
 {
 	struct object *obj = lookup_object(r, oid);
 	if (!obj)
-		return create_object(r, oid, alloc_blob_node(r));
+		return create_object(r, oid, mem_pool_alloc_blob_node(r));
 	return object_as_type(obj, OBJ_BLOB, 0);
 }
 
diff --git a/commit-graph.c b/commit-graph.c
index f3486ec18f1..216d770eeb9 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2362,7 +2362,7 @@  int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
 		hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
 
 		graph_commit = lookup_commit(r, &cur_oid);
-		odb_commit = (struct commit *)create_object(r, &cur_oid, alloc_commit_node(r));
+		odb_commit = (struct commit *)create_object(r, &cur_oid, mem_pool_alloc_commit_node(r));
 		if (parse_commit_internal(odb_commit, 0, 0)) {
 			graph_report(_("failed to parse commit %s from object database for commit-graph"),
 				     oid_to_hex(&cur_oid));
diff --git a/commit.c b/commit.c
index bab8d5ab07c..cd9cd8f94f9 100644
--- a/commit.c
+++ b/commit.c
@@ -61,7 +61,7 @@  struct commit *lookup_commit(struct repository *r, const struct object_id *oid)
 {
 	struct object *obj = lookup_object(r, oid);
 	if (!obj)
-		return create_object(r, oid, alloc_commit_node(r));
+		return create_object(r, oid, mem_pool_alloc_commit_node(r));
 	return object_as_type(obj, OBJ_COMMIT, 0);
 }
 
diff --git a/mem-pool.c b/mem-pool.c
index 8401761dda0..d7bae84a982 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -35,6 +35,11 @@  static struct mp_block *mem_pool_alloc_block(struct mem_pool *pool,
 	return p;
 }
 
+struct mem_pool *mem_pool_new(void)
+{
+	return xmalloc(sizeof(struct mem_pool));
+}
+
 void mem_pool_init(struct mem_pool *pool, size_t initial_size)
 {
 	memset(pool, 0, sizeof(*pool));
@@ -69,6 +74,7 @@  void *mem_pool_alloc(struct mem_pool *pool, size_t len)
 	struct mp_block *p = NULL;
 	void *r;
 
+	pool->alloc_count++;
 	/* round up to a 'uintmax_t' alignment */
 	if (len & (sizeof(uintmax_t) - 1))
 		len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));
diff --git a/mem-pool.h b/mem-pool.h
index fe7507f022b..64529e136cf 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -19,8 +19,16 @@  struct mem_pool {
 
 	/* The total amount of memory allocated by the pool. */
 	size_t pool_alloc;
+
+	/* The count of calling mem_pool_alloc .*/
+	size_t alloc_count;
 };
 
+/*
+ * Create a new mem_pool.
+ */
+struct mem_pool *mem_pool_new(void);
+
 /*
  * Initialize mem_pool with specified initial size.
  */
diff --git a/merge-ort.c b/merge-ort.c
index d36a92b59b7..880a300c35c 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -1774,7 +1774,7 @@  static struct commit *make_virtual_commit(struct repository *repo,
 					  struct tree *tree,
 					  const char *comment)
 {
-	struct commit *commit = alloc_commit_node(repo);
+	struct commit *commit = mem_pool_alloc_commit_node(repo);
 
 	set_merge_remote_desc(commit, comment, (struct object *)commit);
 	set_commit_tree(commit, tree);
diff --git a/merge-recursive.c b/merge-recursive.c
index b052974f191..49ca817d727 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -216,7 +216,7 @@  static struct commit *make_virtual_commit(struct repository *repo,
 					  struct tree *tree,
 					  const char *comment)
 {
-	struct commit *commit = alloc_commit_node(repo);
+	struct commit *commit = mem_pool_alloc_commit_node(repo);
 
 	set_merge_remote_desc(commit, comment, (struct object *)commit);
 	set_commit_tree(commit, tree);
diff --git a/object.c b/object.c
index 98017bed8ef..ec3b656188b 100644
--- a/object.c
+++ b/object.c
@@ -182,7 +182,7 @@  struct object *lookup_unknown_object(const struct object_id *oid)
 	struct object *obj = lookup_object(the_repository, oid);
 	if (!obj)
 		obj = create_object(the_repository, oid,
-				    alloc_object_node(the_repository));
+				mem_pool_alloc_object_node(the_repository));
 	return obj;
 }
 
@@ -471,11 +471,16 @@  struct parsed_object_pool *parsed_object_pool_new(void)
 	struct parsed_object_pool *o = xmalloc(sizeof(*o));
 	memset(o, 0, sizeof(*o));
 
-	o->blob_state = allocate_alloc_state();
-	o->tree_state = allocate_alloc_state();
-	o->commit_state = allocate_alloc_state();
-	o->tag_state = allocate_alloc_state();
-	o->object_state = allocate_alloc_state();
+	o->blob_pool = mem_pool_new();
+	o->tree_pool = mem_pool_new();
+	o->commit_pool = mem_pool_new();
+	o->tag_pool = mem_pool_new();
+	o->object_pool = mem_pool_new();
+	mem_pool_init(o->blob_pool,0);
+	mem_pool_init(o->tree_pool,0);
+	mem_pool_init(o->commit_pool,0);
+	mem_pool_init(o->tag_pool,0);
+	mem_pool_init(o->object_pool,0);
 
 	o->is_shallow = -1;
 	o->shallow_stat = xcalloc(1, sizeof(*o->shallow_stat));
@@ -568,16 +573,16 @@  void parsed_object_pool_clear(struct parsed_object_pool *o)
 	free_commit_buffer_slab(o->buffer_slab);
 	o->buffer_slab = NULL;
 
-	clear_alloc_state(o->blob_state);
-	clear_alloc_state(o->tree_state);
-	clear_alloc_state(o->commit_state);
-	clear_alloc_state(o->tag_state);
-	clear_alloc_state(o->object_state);
+	mem_pool_discard(o->blob_pool,1);
+	mem_pool_discard(o->tree_pool,1);
+	mem_pool_discard(o->tag_pool,1);
+	mem_pool_discard(o->object_pool,1);
+	mem_pool_discard(o->commit_pool,1);
 	stat_validity_clear(o->shallow_stat);
-	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);
+	FREE_AND_NULL(o->blob_pool);
+	FREE_AND_NULL(o->tree_pool);
+	FREE_AND_NULL(o->commit_pool);
+	FREE_AND_NULL(o->tag_pool);
+	FREE_AND_NULL(o->object_pool);
 	FREE_AND_NULL(o->shallow_stat);
 }
diff --git a/object.h b/object.h
index 59daadce214..43031d8dc04 100644
--- a/object.h
+++ b/object.h
@@ -10,11 +10,11 @@  struct parsed_object_pool {
 	int nr_objs, obj_hash_size;
 
 	/* TODO: migrate alloc_states to mem-pool? */
-	struct alloc_state *blob_state;
-	struct alloc_state *tree_state;
-	struct alloc_state *commit_state;
-	struct alloc_state *tag_state;
-	struct alloc_state *object_state;
+	struct mem_pool *blob_pool;
+	struct mem_pool *tree_pool;
+	struct mem_pool *commit_pool;
+	struct mem_pool *tag_pool;
+	struct mem_pool *object_pool;
 
 	/* parent substitutions from .git/info/grafts and .git/shallow */
 	struct commit_graft **grafts;
diff --git a/tag.c b/tag.c
index 1ed2684e45b..de24b6308b4 100644
--- a/tag.c
+++ b/tag.c
@@ -102,7 +102,7 @@  struct tag *lookup_tag(struct repository *r, const struct object_id *oid)
 {
 	struct object *obj = lookup_object(r, oid);
 	if (!obj)
-		return create_object(r, oid, alloc_tag_node(r));
+		return create_object(r, oid, mem_pool_alloc_tag_node(r));
 	return object_as_type(obj, OBJ_TAG, 0);
 }
 
diff --git a/tree.c b/tree.c
index a52479812ce..a4b081a42b7 100644
--- a/tree.c
+++ b/tree.c
@@ -199,7 +199,7 @@  struct tree *lookup_tree(struct repository *r, const struct object_id *oid)
 {
 	struct object *obj = lookup_object(r, oid);
 	if (!obj)
-		return create_object(r, oid, alloc_tree_node(r));
+		return create_object(r, oid, mem_pool_alloc_tree_node(r));
 	return object_as_type(obj, OBJ_TREE, 0);
 }