object store: alloc
cover object store: alloc
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 protected]/
Any comments welcome,
Thanks,
Stefan
Jonathan Nieder (1):
object: add repository argument to grow_object_hash
Stefan Beller (12):
repository: introduce object parser 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 | 69 +++++++++++++++++++++++------------ alloc.h | 21 +++++++++++ blame.c | 3 +- blob.c | 5 ++- cache.h | 9 ----- commit.c | 4 +- merge-recursive.c | 3 +- object.c | 93 +++++++++++++++++++++++++++++++++-------------- object.h | 18 ++++++++- repository.c | 7 ++++ repository.h | 11 +++++- tag.c | 4 +- tree.c | 4 +- 13 files changed, 182 insertions(+), 69 deletions(-) create mode 100644 alloc.h
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 1/13 repository: introduce object parser field
Git's object access code can be thought of as containing two layers:
the raw object store provides access to raw object content, while the higher level obj_hash code parses raw objects and keeps track of parenthood and other object relationships using 'struct object'. Keeping these layers separate should make it easier to find relevant functions and to change the implementation of one without having to touch the other.
Add an object_parser field to 'struct repository' to prepare obj_hash to be handled per repository. Callers still only use the_repository for now --- later patches will adapt them to handle arbitrary repositories.
--- object.c | 63 +++++++++++++++++++++++++++++++++------------------- object.h | 8 +++++++ repository.c | 7 ++++++ repository.h | 11 ++++++++- 4 files changed, 65 insertions(+), 24 deletions(-)
object.c +40 -23
@@ -8,17 +8,14 @@#include "object-store.h" #include "packfile.h" -static struct object **obj_hash; -static int nr_objs, obj_hash_size; -unsigned int get_max_object_index (void ) { -return obj_hash_size; +return the_repository->parsed_objects->obj_hash_size; }struct object *get_indexed_object (unsigned int idx) { -return obj_hash[idx]; +return the_repository->parsed_objects->obj_hash[idx]; }static const char *object_type_strings[] = {
@@ -90,15 +87,16 @@ struct object *lookup_object(const unsigned char *sha1)unsigned int i, first;struct object *obj; -if (!obj_hash) +if (!the_repository->parsed_objects->obj_hash)return NULL ; - first = i =hash_obj (sha1, obj_hash_size); -while ((obj = obj_hash[i]) !=NULL ) { + first = i =hash_obj (sha1, + the_repository->parsed_objects->obj_hash_size); +while ((obj = the_repository->parsed_objects->obj_hash[i]) !=NULL ) {if (!hashcmp (sha1, obj->oid.hash))break ; i++; -if (i == obj_hash_size) +if (i == the_repository->parsed_objects->obj_hash_size) i =0 ; }if (obj && i != first) {
@@ -107,7 +105,8 @@ struct object *lookup_object(const unsigned char *sha1)* that we do not need to walk the hash table the next * time we look for it. */ -SWAP (obj_hash[i], obj_hash[first]); +SWAP (the_repository->parsed_objects->obj_hash[i], + the_repository->parsed_objects->obj_hash[first]); }return obj; }
@@ -124,19 +123,19 @@ static void grow_object_hash(void)* Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = obj_hash_size <32 ?32 :2 * obj_hash_size; +int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (obj_hash); - obj_hash = new_hash; - obj_hash_size = new_hash_size; +free (the_repository->parsed_objects->obj_hash); + the_repository->parsed_objects->obj_hash = new_hash; + the_repository->parsed_objects->obj_hash_size = new_hash_size; }void *create_object (const unsigned char *sha1,void *o)
@@ -147,11 +146,12 @@ void *create_object(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (obj_hash_size -1 <= nr_objs *2 ) +if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 )grow_object_hash (); -insert_obj_hash (obj, obj_hash, obj_hash_size); - nr_objs++; +insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, + the_repository->parsed_objects->obj_hash_size); + the_repository->parsed_objects->nr_objs++;return obj; }
@@ -431,8 +431,8 @@ void clear_object_flags(unsigned flags) {int i; -for (i=0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i=0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj) obj->flags &= ~flags; }
@@ -442,13 +442,20 @@ void clear_commit_marks_all(unsigned int flags) {int i; -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj && obj->type ==OBJ_COMMIT ) obj->flags &= ~flags; } } +struct object_parser *object_parser_new (void ) +{ +struct object_parser *o =xmalloc (sizeof (*o)); +memset (o,0 ,sizeof (*o)); +return o; +} +struct raw_object_store *raw_object_store_new (void ) {struct raw_object_store *o =xmalloc (sizeof (*o));
@@ -488,3 +495,13 @@ void raw_object_store_clear(struct raw_object_store *o)close_all_packs (o); o->packed_git =NULL ; } + +void object_parser_clear (struct object_parser *o) +{ +/* +* TOOD free objects in o->obj_hash. +* +* As objects are allocated in slabs (see alloc.c), we do +* not need to free each object, but each slab instead. +*/ +}
object.h +8 -0
@@ -1,6 +1,14 @@ #ifndef OBJECT_H #define OBJECT_H +struct object_parser { + struct object **obj_hash; + int nr_objs, obj_hash_size; +}; + +struct object_parser *object_parser_new(void); +void object_parser_clear(struct object_parser *o); + struct object_list { struct object *item; struct object_list *next;
repository.c +7 -0
@@ -2,6 +2,7 @@#include "repository.h" #include "object-store.h" #include "config.h" +#include "object.h" #include "submodule-config.h" /* The main repository */
@@ -14,6 +15,8 @@ void initialize_the_repository(void) the_repo.index = &the_index; the_repo.objects =raw_object_store_new (); + the_repo.parsed_objects =object_parser_new (); +repo_set_hash_algo (&the_repo,GIT_HASH_SHA1 ); }
@@ -143,6 +146,7 @@ static int repo_init(struct repository *repo,memset (repo,0 ,sizeof (*repo)); repo->objects =raw_object_store_new (); + repo->parsed_objects =object_parser_new ();if (repo_init_gitdir (repo, gitdir)) goto error;
@@ -226,6 +230,9 @@ void repo_clear(struct repository *repo)raw_object_store_clear (repo->objects);FREE_AND_NULL (repo->objects); +object_parser_clear (repo->parsed_objects); +FREE_AND_NULL (repo->parsed_objects); +if (repo->config) {git_configset_clear (repo->config);FREE_AND_NULL (repo->config);
repository.h +10 -1
@@ -22,10 +22,19 @@ struct repository {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; +/* +* State for the object parser. This owns all parsed objects +* (struct object) so callers do not have to manage their +* lifetime. +*/ +struct object_parser *parsed_objects; +/* The store in which the refs are held. */ struct ref_store *refs;
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH 01/13] repository: introduce object parser field
Quoted 4 lines · Stefan Beller
/*repository.h +2
-1-* 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
repository.h struct repository { · line 36
Quoted 10 lines
repository.h +7
-0+* content which it then parses. */ struct raw_object_store *objects; +/* +* State for the object parser. This owns all parsed objects +* (struct object) so callers do not have to manage their +* lifetime. +*/ +struct object_parser *parsed_objects;
I like this name 'parsed_objects'. Should we rename the struct after it (e.g. parsed_object_store as opposed to raw_object_store above)?
Another suggestion is object_pool, if we keep 'struct object' instead of 'struct parsed_object' and also want to keep current allocation
behavior: no individual deallocation. If you free, you free the whole pool (e.g. you could run rev-list --all --objects in a separate pool, once you're done, you delete the whole pool).
Signature · 1 line
Duy
Re: [PATCH 01/13] repository: introduce object parser field
Quoted 16 lines · Duy Nguyen
Quoted 4 lines · Stefan Beller
/*repository.h +2
-1-* 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 repository.h struct repository { · line 36
Quoted 10 lines
repository.h +7
-0+* content which it then parses. */ struct raw_object_store *objects; +/* +* State for the object parser. This owns all parsed objects +* (struct object) so callers do not have to manage their +* lifetime. +*/ +struct object_parser *parsed_objects;
Quoted 2 lines
I like this name '
parsed_objects'. Should we rename the struct after it (e.g.parsed_object_storeas opposed toraw_object_storeabove)?
I can rename it to parsed_object_store for consistency.
Quoted 5 lines
Another suggestion is
object_pool, if we keep 'struct object' instead of 'structparsed_object' and also want to keep current allocation
behavior: no individual deallocation. If you free, you free the whole pool (e.g. you could run rev-list --all --objects in a separate pool, once you're done, you delete the whole pool).
That is what the following patches will be about, you can only free the whole set of parsed objects.
So if you want to do a separate rev walk, you may need to instantiate a new repository for it (ideally you'd only need a separate parsed object store).
I'd want to have the ability to have separate pools for submodules, such that they can be free'd on a per-repo basis.
Quoted 2 lines
--
Duy
Re: [PATCH 01/13] repository: introduce object parser field
Quoted 6 lines · Stefan Beller
Quoted 5 lines
Another suggestion is
object_pool, if we keep 'struct object' instead of 'structparsed_object' and also want to keep current allocation
behavior: no individual deallocation. If you free, you free the whole pool (e.g. you could run rev-list --all --objects in a separate pool, once you're done, you delete the whole pool).
Quoted 6 lines
That is what the following patches will be about, you can only free the whole set of parsed objects.
So if you want to do a separate rev walk, you may need to instantiate a new repository for it (ideally you'd only need a separate parsed object store).
I'm not sure if it's a good idea to create a separate struct repository just because you want to free this parsed object store. What if updates are made in both repositories? All the cache (well, mostly the delta base cache in sha1_file.c) will double memory usage as well. Yeah not ideal. But I guess making rev-list related code use a separate parsed object store is no small task (and kinda risky as well since we migrate from global lookup_* functions to local ones and need to choose the correct parsed object store to look up from)
For your information, there is already a good use case for this wholesale memory free: if we can free the rev-list related memory early in pack-objects (e.g. part of repack operation) then it could lower memory pressure significantly when running on large repos. This has been discussed a bit lately.
Signature · 1 line
Duy
Re: [PATCH 01/13] repository: introduce object parser field
In reply to Stefan BellerQuoted 7 lines · On Tue, 1 May 2018 14:33:51 -0700 Stefan Beller <[email protected]> wrote:
Git's object access code can be thought of as containing two layers:
the raw object store provides access to raw object content, while the higher levelobj_hashcode parses raw objects and keeps track of parenthood and other object relationships using 'struct object'. Keeping these layers separate should make it easier to find relevant functions and to change the implementation of one without having to touch the other.
I only understood this after reading the code below. I think this paragraph can be removed; I don't see its relevance - of course we need to store metadata about how to load objects somewhere, and caching objects we have already loaded is a good idea: and the metadata and cache are two separate things before and after this patch anyway.
Quoted 4 lines
Add an
object_parserfield to 'struct repository' to prepareobj_hashto be handled per repository. Callers still only usethe_repositoryfor now --- later patches will adapt them to handle arbitrary repositories.
I think this is better reworded as:
Convert the existing global cache for parsed objects (obj_hash) into repository-specific parsed object caches. Existing code that uses obj_hash are modified to use the parsed object cache of the_repository; future patches will use the parsed object caches of other repositories.
object.c void clear_commit_marks_all(unsigned int flags) · line 457
Quoted 6 lines
object.c +6
-0+struct object_parser *object_parser_new (void ) +{ +struct object_parser *o =xmalloc (sizeof (*o)); +memset (o,0 ,sizeof (*o)); +return o; +}
I'm not sure that I like this style of code (I prefer the strbuf style with _INIT and release(), which I think is more flexible) but I don't feel too strongly about it.
object.h line 7
Quoted 4 lines
object.h +4
-0+struct object_parser { +struct object **obj_hash; +int nr_objs, obj_hash_size; +};
object_parser is probably a bad name. I think Duy had some good suggestions in [1].
[1] https://public-inbox.org/git/CACsJy8CgX6BME=EhiDBfMRzBOYDBNHE6Ouxv4fZC-GW7Rsi81Q@mail.gmail.com/
repository.h struct repository { · line 29
Quoted 7 lines
repository.h +3
-1/* -* 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;
I think the additional sentence ("The object_parser uses this...") is unnecessary and confusing, especially if its identity is going to be one of storage (like "parsed_objects" implies).
repository.h struct repository { · line 36
Quoted 6 lines
repository.h +6
-0+/* +* State for the object parser. This owns all parsed objects +* (struct object) so callers do not have to manage their +* lifetime. +*/ +struct object_parser *parsed_objects;
Even after all the commits in this patch set, this does not store any state for parsing. Maybe just document as:
All objects in this repository that have been parsed. This structure owns all objects it references, so users of "struct object *" generally do not need to free them; instead, when a repository is no longer used, call object_parser_clear() on this structure.
(And maybe say that the freeing method on struct repository will automatically call object_parser_clear().)
PATCH 2/13 object: add repository argument to create_object
In reply to Stefan BellerAdd a repository argument to allow the callers of create_object to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
Add the cocci patch that converted the callers.
--- blob.c | 4 +++- commit.c | 3 ++- object.c | 5 +++-- object.h | 3 ++- tag.c | 3 ++- tree.c | 3 ++- 6 files changed, 14 insertions(+), 7 deletions(-)
blob.c +3 -1
@@ -1,5 +1,6 @@#include "cache.h" #include "blob.h" +#include "repository.h" const char *blob_type ="blob" ;
@@ -7,7 +8,8 @@ struct blob *lookup_blob(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_blob_node ()); +return create_object (the_repository, oid->hash, +alloc_blob_node ());return object_as_type (obj,OBJ_BLOB ,0 ); }
commit.c +2 -1
@@ -50,7 +50,8 @@ struct commit *lookup_commit(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_commit_node ()); +return create_object (the_repository, oid->hash, +alloc_commit_node ());return object_as_type (obj,OBJ_COMMIT ,0 ); }
object.c +3 -2
@@ -138,7 +138,7 @@ static void grow_object_hash(void) the_repository->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object (const unsigned char *sha1,void *o) +void *create_object_the_repository (const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -178,7 +178,8 @@ struct object *lookup_unknown_object(const unsigned char *sha1) {struct object *obj =lookup_object (sha1);if (!obj) - obj =create_object (sha1,alloc_object_node ()); + obj =create_object (the_repository, sha1, +alloc_object_node ());return obj; }
object.h +2 -1
@@ -93,7 +93,8 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -extern void *create_object (const unsigned char *sha1,void *obj); +#define create_object (r, s, o) create_object_##r(s, o) +extern void *create_object_the_repository (const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
tag.c +2 -1
@@ -93,7 +93,8 @@ struct tag *lookup_tag(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tag_node ()); +return create_object (the_repository, oid->hash, +alloc_tag_node ());return object_as_type (obj,OBJ_TAG ,0 ); }
tree.c +2 -1
@@ -196,7 +196,8 @@ struct tree *lookup_tree(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tree_node ()); +return create_object (the_repository, oid->hash, +alloc_tree_node ());return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH 02/13] object: add repository argument to create_object
Quoted 7 lines · Stefan Beller
Add a repository argument to allow the callers of
create_objectto be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other thanthe_repositoryyet.As with the previous commits, use a macro to catch callers passing a repository other than
the_repositoryat compile time.
This is only patch 2, and patch 1 did not use a macro trick to catch callers passing the wrong argument.
Quoted 1 line
Add the cocci patch that converted the callers.
No cocci patch anywhere in sight.
Quoted 2 lines
Signed-off-by Jonathan Nieder <[email protected]>Signed-off-by Stefan Beller <[email protected]>
PATCH 3/13 object: add repository argument to grow_object_hash
In reply to Stefan BellerFrom: Jonathan Nieder <[email protected]>
Add a repository argument to allow the caller of grow_object_hash to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- object.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
object.c +3 -2
@@ -116,7 +116,8 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -static void grow_object_hash (void ) +#define grow_object_hash (r) grow_object_hash_##r() +static void grow_object_hash_the_repository (void ) {int i;/*
@@ -147,7 +148,7 @@ void *create_object_the_repository(const unsigned char *sha1, void *o)hashcpy (obj->oid.hash, sha1);if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (); +grow_object_hash (the_repository);insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, the_repository->parsed_objects->obj_hash_size);
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 4/13 alloc: add repository argument to alloc_blob_node
In reply to Stefan Beller--- alloc.c | 2 +- blob.c | 2 +- cache.h | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -49,7 +49,7 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size) }static struct alloc_state blob_state; -void *alloc_blob_node (void ) +void *alloc_blob_node_the_repository (void ) {struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;
blob.c +1 -1
@@ -9,7 +9,7 @@ struct blob *lookup_blob(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_blob_node ()); +alloc_blob_node (the_repository));return object_as_type (obj,OBJ_BLOB ,0 ); }
cache.h +2 -1
@@ -1764,7 +1764,8 @@ int decode_85(char *dst, const char *line, int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes);/* alloc.c */ -extern void *alloc_blob_node (void ); +#define alloc_blob_node (r) alloc_blob_node_##r() +extern void *alloc_blob_node_the_repository (void );extern void *alloc_tree_node (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH 04/13] alloc: add repository argument to alloc_blob_node
Quoted 1 line · On Tue, 1 May 2018 14:33:54 -0700 Stefan Beller <[email protected]> wrote:
Signed-off-by Stefan Beller <[email protected]>
Add the same boilerplate explanation to this and subsequent commits. If editing it for every new function name is cumbersome, maybe use this shortened version:
This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
PATCH 5/13 alloc: add repository argument to alloc_tree_node
In reply to Stefan Beller--- alloc.c | 2 +- cache.h | 3 ++- tree.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -57,7 +57,7 @@ void *alloc_blob_node_the_repository(void) }static struct alloc_state tree_state; -void *alloc_tree_node (void ) +void *alloc_tree_node_the_repository (void ) {struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;
cache.h +2 -1
@@ -1766,7 +1766,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);/* alloc.c */ #define alloc_blob_node (r) alloc_blob_node_##r()extern void *alloc_blob_node_the_repository (void ); -extern void *alloc_tree_node (void ); +#define alloc_tree_node (r) alloc_tree_node_##r() +extern void *alloc_tree_node_the_repository (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );
tree.c +1 -1
@@ -197,7 +197,7 @@ struct tree *lookup_tree(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tree_node ()); +alloc_tree_node (the_repository));return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 6/13 alloc: add repository argument to alloc_commit_node
In reply to Stefan Beller--- alloc.c | 2 +- blame.c | 2 +- cache.h | 3 ++- commit.c | 2 +- merge-recursive.c | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-)
alloc.c +1 -1
@@ -88,7 +88,7 @@ unsigned int alloc_commit_index(void)return count++; } -void *alloc_commit_node (void ) +void *alloc_commit_node_the_repository (void ) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ;
blame.c +1 -1
@@ -161,7 +161,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,read_cache ();time (&now); - commit =alloc_commit_node (); + commit =alloc_commit_node (the_repository); commit->object.parsed =1 ; commit->date = now; parent_tail = &commit->parents;
cache.h +2 -1
@@ -1768,7 +1768,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);extern void *alloc_blob_node_the_repository (void );#define alloc_tree_node (r) alloc_tree_node_##r()extern void *alloc_tree_node_the_repository (void ); -extern void *alloc_commit_node (void ); +#define alloc_commit_node (r) alloc_commit_node_##r() +extern void *alloc_commit_node_the_repository (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );extern void alloc_report (void );
commit.c +1 -1
@@ -51,7 +51,7 @@ struct commit *lookup_commit(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_commit_node ()); +alloc_commit_node (the_repository));return object_as_type (obj,OBJ_COMMIT ,0 ); }
merge-recursive.c +1 -1
@@ -98,7 +98,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two,static struct commit *make_virtual_commit (struct tree *tree,const char *comment) { -struct commit *commit =alloc_commit_node (); +struct commit *commit =alloc_commit_node (the_repository);set_merge_remote_desc (commit, comment, (struct object *)commit); commit->tree = tree;
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 7/13 alloc: add repository argument to alloc_tag_node
In reply to Stefan Beller--- alloc.c | 2 +- cache.h | 3 ++- tag.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -65,7 +65,7 @@ void *alloc_tree_node_the_repository(void) }static struct alloc_state tag_state; -void *alloc_tag_node (void ) +void *alloc_tag_node_the_repository (void ) {struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;
cache.h +2 -1
@@ -1770,7 +1770,8 @@ extern void *alloc_blob_node_the_repository(void);extern void *alloc_tree_node_the_repository (void );#define alloc_commit_node (r) alloc_commit_node_##r()extern void *alloc_commit_node_the_repository (void ); -extern void *alloc_tag_node (void ); +#define alloc_tag_node (r) alloc_tag_node_##r() +extern void *alloc_tag_node_the_repository (void );extern void *alloc_object_node (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
tag.c +1 -1
@@ -94,7 +94,7 @@ struct tag *lookup_tag(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tag_node ()); +alloc_tag_node (the_repository));return object_as_type (obj,OBJ_TAG ,0 ); }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 8/13 alloc: add repository argument to alloc_object_node
In reply to Stefan Beller--- alloc.c | 2 +- cache.h | 3 ++- object.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -73,7 +73,7 @@ void *alloc_tag_node_the_repository(void) }static struct alloc_state object_state; -void *alloc_object_node (void ) +void *alloc_object_node_the_repository (void ) {struct object *obj =alloc_node (&object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;
cache.h +2 -1
@@ -1772,7 +1772,8 @@ extern void *alloc_tree_node_the_repository(void);extern void *alloc_commit_node_the_repository (void );#define alloc_tag_node (r) alloc_tag_node_##r()extern void *alloc_tag_node_the_repository (void ); -extern void *alloc_object_node (void ); +#define alloc_object_node (r) alloc_object_node_##r() +extern void *alloc_object_node_the_repository (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
object.c +1 -1
@@ -180,7 +180,7 @@ struct object *lookup_unknown_object(const unsigned char *sha1)struct object *obj =lookup_object (sha1);if (!obj) obj =create_object (the_repository, sha1, -alloc_object_node ()); +alloc_object_node (the_repository));return obj; }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 9/13 alloc: add repository argument to alloc_report
In reply to Stefan Beller--- alloc.c | 2 +- cache.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
alloc.c +1 -1
@@ -105,7 +105,7 @@ static void report(const char *name, unsigned int count, size_t size)#define REPORT (name, type) \ report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) -void alloc_report (void ) +void alloc_report_the_repository (void ) { REPORT(blob, struct blob); REPORT(tree, struct tree);
cache.h +2 -1
@@ -1774,7 +1774,8 @@ extern void *alloc_commit_node_the_repository(void);extern void *alloc_tag_node_the_repository (void );#define alloc_object_node (r) alloc_object_node_##r()extern void *alloc_object_node_the_repository (void ); -extern void alloc_report (void ); +#define alloc_report (r) alloc_report_##r() +extern void alloc_report_the_repository (void );extern unsigned int alloc_commit_index (void );/* pkt-line.c */
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 10/13 alloc: add repository argument to alloc_commit_index
In reply to Stefan Beller--- alloc.c | 4 ++-- cache.h | 3 ++- object.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-)
alloc.c +2 -2
@@ -82,7 +82,7 @@ void *alloc_object_node_the_repository(void)static struct alloc_state commit_state; -unsigned int alloc_commit_index (void ) +unsigned int alloc_commit_index_the_repository (void ) {static unsigned int count;return count++;
@@ -92,7 +92,7 @@ void *alloc_commit_node_the_repository(void) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (); + c->index =alloc_commit_index (the_repository);return c; }
cache.h +2 -1
@@ -1776,7 +1776,8 @@ extern void *alloc_tag_node_the_repository(void);extern void *alloc_object_node_the_repository (void );#define alloc_report (r) alloc_report_##r()extern void alloc_report_the_repository (void ); -extern unsigned int alloc_commit_index (void ); +#define alloc_commit_index (r) alloc_commit_index_##r() +extern unsigned int alloc_commit_index_the_repository (void );/* pkt-line.c */ void packet_trace_identity (const char *prog);
object.c +1 -1
@@ -162,7 +162,7 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet)return obj; else if (obj->type == OBJ_NONE) {if (type ==OBJ_COMMIT ) - ((struct commit *)obj)->index =alloc_commit_index (); + ((struct commit *)obj)->index =alloc_commit_index (the_repository); obj->type = type;return obj; }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 11/13 object: allow grow_object_hash to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
object.c +8 -8
@@ -116,27 +116,27 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -#define grow_object_hash (r) grow_object_hash_##r() -static void grow_object_hash_the_repository (void ) +static void grow_object_hash (struct repository *r) {int i;/* * Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size; +int new_hash_size = r->parsed_objects->obj_hash_size <32 ?32 :2 * r->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { -struct object *obj = the_repository->parsed_objects->obj_hash[i]; +for (i =0 ; i < r->parsed_objects->obj_hash_size; i++) { +struct object *obj = r->parsed_objects->obj_hash[i]; +if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (the_repository->parsed_objects->obj_hash); - the_repository->parsed_objects->obj_hash = new_hash; - the_repository->parsed_objects->obj_hash_size = new_hash_size; +free (r->parsed_objects->obj_hash); + r->parsed_objects->obj_hash = new_hash; + r->parsed_objects->obj_hash_size = new_hash_size; }void *create_object_the_repository (const unsigned char *sha1,void *o)
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 12/13 object: allow create_object to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 12 ++++++------ object.h | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-)
object.c +6 -6
@@ -139,7 +139,7 @@ static void grow_object_hash(struct repository *r) r->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object_the_repository (const unsigned char *sha1,void *o) +void *create_object (struct repository *r,const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -147,12 +147,12 @@ void *create_object_the_repository(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (the_repository); +if (r->parsed_objects->obj_hash_size -1 <= r->parsed_objects->nr_objs *2 ) +grow_object_hash (r); -insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, - the_repository->parsed_objects->obj_hash_size); - the_repository->parsed_objects->nr_objs++; +insert_obj_hash (obj, r->parsed_objects->obj_hash, + r->parsed_objects->obj_hash_size); + r->parsed_objects->nr_objs++;return obj; }
object.h +1 -2
@@ -93,8 +93,7 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -#define create_object (r, s, o) create_object_##r(s, o) -extern void *create_object_the_repository (const unsigned char *sha1,void *obj); +extern void *create_object (struct repository *r,const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH 12/13] object: allow create_object to handle arbitrary repositories
Quoted 2 lines · On Tue, 1 May 2018 14:34:02 -0700 Stefan Beller <[email protected]> wrote:
Signed-off-by Jonathan Nieder <[email protected]>Signed-off-by Stefan Beller <[email protected]>
Downloading this patch set and viewing the whole function modified in this patch shows that globals are no longer referenced, so this is good. Same comment for patch 11.
PATCH 13/13 alloc: allow arbitrary repositories for alloc functions
In reply to Stefan BellerWe have to convert all of the alloc functions at once, because alloc_report uses a funky macro for reporting. It is better for the sake of mechanical conversion to convert multiple functions at once rather than changing the structure of the reporting function.
We record all memory allocation in alloc.c, and free them in clear_alloc_state, which is called for all repositories except the_repository.
--- alloc.c | 69 ++++++++++++++++++++++++++++++----------------- alloc.h | 21 +++++++++++++++ blame.c | 1 + blob.c | 1 + cache.h | 16 ----------- commit.c | 1 + merge-recursive.c | 1 + object.c | 24 ++++++++++++++--- object.h | 10 ++++++- repository.c | 4 +-- tag.c | 1 + tree.c | 1 + 12 files changed, 104 insertions(+), 46 deletions(-) create mode 100644 alloc.h
alloc.c +45 -24
@@ -4,10 +4,11 @@* Copyright (C) 2006 Linus Torvalds * * The standard malloc/free wastes too much space for objects, partly because -* it maintains all the allocation infrastructure (which isn't needed, since -* we never free an object descriptor anyway), but even more because it ends +* it maintains all the allocation infrastructure, but even more because it ends * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. +* +* TODO: Combine this with mem-pool? */ #include "cache.h" #include "object.h"
@@ -30,8 +31,25 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs; +int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + } +} +static inline void *alloc_node (struct alloc_state *s,size_t node_size) {void *ret;
@@ -45,54 +63,56 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size) ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = ret; +return ret; } -static struct alloc_state blob_state; -void *alloc_blob_node_the_repository (void ) +struct alloc_state the_repository_blob_state; +void *alloc_blob_node (struct repository *r) { -struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); +struct blob *b =alloc_node (r->parsed_objects->blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;return b; } -static struct alloc_state tree_state; -void *alloc_tree_node_the_repository (void ) +struct alloc_state the_repository_tree_state; +void *alloc_tree_node (struct repository *r) { -struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); +struct tree *t =alloc_node (r->parsed_objects->tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;return t; } -static struct alloc_state tag_state; -void *alloc_tag_node_the_repository (void ) +struct alloc_state the_repository_tag_state; +void *alloc_tag_node (struct repository *r) { -struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); +struct tag *t =alloc_node (r->parsed_objects->tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;return t; } -static struct alloc_state object_state; -void *alloc_object_node_the_repository (void ) +struct alloc_state the_repository_object_state; +void *alloc_object_node (struct repository *r) { -struct object *obj =alloc_node (&object_state,sizeof (union any_object )); +struct object *obj =alloc_node (r->parsed_objects->object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;return obj; } -static struct alloc_state commit_state; - -unsigned int alloc_commit_index_the_repository (void ) +unsigned int alloc_commit_index (struct repository *r) { -static unsigned int count; -return count++; +return r->parsed_objects->commit_count++; } -void *alloc_commit_node_the_repository (void ) +struct alloc_state the_repository_commit_state; +void *alloc_commit_node (struct repository *r) { -struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); +struct commit *c =alloc_node (r->parsed_objects->commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (the_repository); + c->index =alloc_commit_index (r);return c; }
@@ -103,9 +123,10 @@ static void report(const char *name, unsigned int count, size_t size) }#define REPORT (name, type) \ - report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) + report(#name, r->parsed_objects->name##_state->count, \ + r->parsed_objects->name##_state->count * sizeof(type) >> 10) -void alloc_report_the_repository (void ) +void alloc_report (struct repository *r) { REPORT(blob, struct blob); REPORT(tree, struct tree);
alloc.h new file +21 -0
@@ -0,0 +1,21 @@ +#ifndef ALLOC_H +#define ALLOC_H + +void *alloc_blob_node (struct repository *r); +void *alloc_tree_node (struct repository *r); +void *alloc_commit_node (struct repository *r); +void *alloc_tag_node (struct repository *r); +void *alloc_object_node (struct repository *r); +void alloc_report (struct repository *r); +unsigned int alloc_commit_index (struct repository *r); + +void *allocate_alloc_state (void ); +void clear_alloc_state (struct alloc_state *s); + +extern struct alloc_state the_repository_blob_state; +extern struct alloc_state the_repository_tree_state; +extern struct alloc_state the_repository_commit_state; +extern struct alloc_state the_repository_tag_state; +extern struct alloc_state the_repository_object_state; + +#endif
blame.c +1 -0
@@ -6,6 +6,7 @@#include "diffcore.h" #include "tag.h" #include "blame.h" +#include "alloc.h" void blame_origin_decref (struct blame_origin *o) {
blob.c +1 -0
@@ -1,6 +1,7 @@#include "cache.h" #include "blob.h" #include "repository.h" +#include "alloc.h" const char *blob_type ="blob" ;
cache.h +0 -16
@@ -1763,22 +1763,6 @@ extern const char *excludes_file;int decode_85 (char *dst,const char *line,int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes); -/* alloc.c */ -#define alloc_blob_node (r) alloc_blob_node_##r() -extern void *alloc_blob_node_the_repository (void ); -#define alloc_tree_node (r) alloc_tree_node_##r() -extern void *alloc_tree_node_the_repository (void ); -#define alloc_commit_node (r) alloc_commit_node_##r() -extern void *alloc_commit_node_the_repository (void ); -#define alloc_tag_node (r) alloc_tag_node_##r() -extern void *alloc_tag_node_the_repository (void ); -#define alloc_object_node (r) alloc_object_node_##r() -extern void *alloc_object_node_the_repository (void ); -#define alloc_report (r) alloc_report_##r() -extern void alloc_report_the_repository (void ); -#define alloc_commit_index (r) alloc_commit_index_##r() -extern unsigned int alloc_commit_index_the_repository (void ); -/* pkt-line.c */ void packet_trace_identity (const char *prog);
commit.c +1 -0
@@ -6,6 +6,7 @@#include "diff.h" #include "revision.h" #include "notes.h" +#include "alloc.h" #include "gpg-interface.h" #include "mergesort.h" #include "commit-slab.h"
merge-recursive.c +1 -0
@@ -14,6 +14,7 @@#include "tree-walk.h" #include "diff.h" #include "diffcore.h" +#include "alloc.h" #include "tag.h" #include "unpack-trees.h" #include "string-list.h"
object.c +21 -3
@@ -4,6 +4,7 @@#include "blob.h" #include "tree.h" #include "commit.h" +#include "alloc.h" #include "tag.h" #include "object-store.h" #include "packfile.h"
@@ -451,10 +452,24 @@ void clear_commit_marks_all(unsigned int flags) } } -struct object_parser *object_parser_new (void ) +struct object_parser *object_parser_new (int is_the_repo) {struct object_parser *o =xmalloc (sizeof (*o));memset (o,0 ,sizeof (*o)); + +if (is_the_repo) { + o->blob_state = &the_repository_blob_state; + o->tree_state = &the_repository_tree_state; + o->commit_state = &the_repository_commit_state; + o->tag_state = &the_repository_tag_state; + o->object_state = &the_repository_object_state; + }else { + 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 (); + }return o; }
@@ -501,9 +516,12 @@ void raw_object_store_clear(struct raw_object_store *o)void object_parser_clear (struct object_parser *o) {/* -* TOOD free objects in o->obj_hash. -* * As objects are allocated in slabs (see alloc.c), we do * not need to free each object, but each slab instead. */ +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); }
object.h +9 -1
@@ -4,9 +4,17 @@struct object_parser {struct object **obj_hash;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; +unsigned commit_count; }; -struct object_parser *object_parser_new (void ); +struct object_parser *object_parser_new (int is_the_repo);void object_parser_clear (struct object_parser *o);struct object_list {
repository.c +2 -2
@@ -15,7 +15,7 @@ void initialize_the_repository(void) the_repo.index = &the_index; the_repo.objects =raw_object_store_new (); - the_repo.parsed_objects =object_parser_new (); + the_repo.parsed_objects =object_parser_new (1 );repo_set_hash_algo (&the_repo,GIT_HASH_SHA1 ); }
@@ -146,7 +146,7 @@ static int repo_init(struct repository *repo,memset (repo,0 ,sizeof (*repo)); repo->objects =raw_object_store_new (); - repo->parsed_objects =object_parser_new (); + repo->parsed_objects =object_parser_new (0 );if (repo_init_gitdir (repo, gitdir)) goto error;
tag.c +1 -0
@@ -3,6 +3,7 @@#include "commit.h" #include "tree.h" #include "blob.h" +#include "alloc.h" #include "gpg-interface.h" const char *tag_type ="tag" ;
tree.c +1 -0
@@ -5,6 +5,7 @@#include "blob.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "tree-walk.h" const char *tree_type ="tree" ;
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions
alloc.c struct alloc_state { · line 36
Quoted 9 lines · Stefan Beller
#include "
cache.h"
#include "object.h"alloc.c +3
-0@@ -30,8 +31,25 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs;
Another way to manage this is linked list: you could reserve one "object" in each slab to store the "next" (or "prev") pointer to another slab, then you can just walk through all slabs and free. It's a bit cheaper than reallocating slabs[], but I guess we reallocate so few times that readability matters more (whichever way is chosen).
alloc.c struct alloc_state { · line 49
Quoted 13 lines
alloc.c +11
-0+int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]);
I think you're leaking memory here. Commit and tree objects may have more allocations in them (especially trees, but I think we have commit_list in struct commit too). Those need to be freed as well.
Quoted 6 lines
alloc.c +3
-0+ } +} +static inline void *alloc_node (struct alloc_state *s,size_t node_size) {void *ret;
Signature · 1 line
Duy
Re: [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 16 lines · Duy Nguyen
alloc.c struct alloc_state { · line 36
Quoted 9 lines · Stefan Beller
#include "
cache.h"
#include "object.h"alloc.c +3
-0@@ -30,8 +31,25 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs;Another way to manage this is linked list: you could reserve one "object" in each slab to store the "next" (or "prev") pointer to another slab, then you can just walk through all slabs and free. It's a bit cheaper than reallocating slabs[], but I guess we reallocate so few times that readability matters more (whichever way is chosen).
This is a good idea. I'll do so in a resend.
Quoted 9 lines
alloc.c struct alloc_state { · line 49
Quoted 5 lines
alloc.c +5
-0+void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]);I think you're leaking memory here. Commit and tree objects may have more allocations in them (especially trees, but I think we have
commit_listin struct commit too). Those need to be freed as well.
I would think that tree and commit memory will be free'd in the parsed_objects release function? (TODO: I need to add it over there)
Thanks,
Stefan
Re: [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 10 lines · Stefan Beller
Quoted 9 lines
alloc.c struct alloc_state { · line 49
Quoted 5 lines
alloc.c +5
-0+void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]);I think you're leaking memory here. Commit and tree objects may have more allocations in them (especially trees, but I think we have
commit_listin struct commit too). Those need to be freed as well.
Quoted 2 lines
I would think that tree and commit memory will be free'd in the
parsed_objectsrelease function? (TODO: I need to add it over there)
What release function? I know tree->buffer is often released since you don't want to keep much in memory. But the user should not be required to release all objects before calling object_parser_clear(). For struct commit, I'm pretty we make the commit_list (for commit parents) and never free. Now we need to do it, somewhere.
Oh you mean object_parser_clear() as release function? Yes. I've been wondering if it makes more sense to go through obj_hash[] and release objects before free(obj_hash) than doing it here. It's probably better to do it there since obj_hash[] would contain the very last pointers to these objects and look like the right place to release resources.
Quoted 2 lines
Thanks,
Stefan
Signature · 1 line
Duy
Re: [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions
In reply to Stefan Belleralloc.c struct alloc_state { · line 51
Quoted 12 lines · On Tue, 1 May 2018 14:34:03 -0700 Stefan Beller <[email protected]> wrote:
alloc.c +12
-0+void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + } +}
These functions are asymmetrical. I understand why it is this way (because we use pointers, and we want to use FREE_AND_NULL), but would still prefer _INIT and _release().
alloc.c static inline void *alloc_node(struct alloc_state *s, size_t node_size) · line 71
Quoted 13 lines
static inline void *alloc_node(struct
alloc_state*s,size_tnode_size) {alloc.c +4
-0void *ret;@@ -45,54 +63,56 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size) ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = ret; +return ret; }
This unconditionally grows the slabs for each node allocation. Shouldn't more than one node fit in each slab?
alloc.h line 19
Quoted 5 lines
alloc.h new file +5
-0+extern struct alloc_state the_repository_blob_state; +extern struct alloc_state the_repository_tree_state; +extern struct alloc_state the_repository_commit_state; +extern struct alloc_state the_repository_tag_state; +extern struct alloc_state the_repository_object_state;
(Context: these were in alloc.h)
These seem to be used only in object.c, and only in one function - could we declare them "static" inside that function instead?
Quoted 11 lines
object.c +7
-1-struct object_parser *object_parser_new (void ) +struct object_parser *object_parser_new (int is_the_repo) {struct object_parser *o =xmalloc (sizeof (*o));memset (o,0 ,sizeof (*o)); + +if (is_the_repo) { + o->blob_state = &the_repository_blob_state; + o->tree_state = &the_repository_tree_state; + o->commit_state = &the_repository_commit_state; + o->tag_state = &the_repository_tag_state;
object.c void clear_commit_marks_all(unsigned int flags) · line 474
Quoted 10 lines
object.c +8
-0+ o->object_state = &the_repository_object_state; + }else { + 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 (); + }return o; }
I don't think saving 5 allocations is worth the code complexity (of the additional parameter).
Re: [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 20 lines · Jonathan Tan
alloc.c struct alloc_state { · line 51
Quoted 12 lines · On Tue, 1 May 2018 14:34:03 -0700 Stefan Beller <[email protected]> wrote:
alloc.c +12
-0+void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + } +}These functions are asymmetrical. I understand why it is this way (because we use pointers, and we want to use
FREE_AND_NULL), but would still prefer_INITand_release().
Quoted 16 lines
alloc.c static inline void *alloc_node(struct alloc_state *s, size_t node_size) · line 71
Quoted 13 lines
static inline void *alloc_node(struct
alloc_state*s,size_tnode_size){ void *ret;alloc.c +4
-0@@ -45,54 +63,56 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size) ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = ret; +return ret; }This unconditionally grows the slabs for each node allocation. Shouldn't more than one node fit in each slab?
Yes. I'll go with Duy's idea and make it a linked list by using the first object as a pointer to the next slab.
Quoted 11 lines
alloc.h line 19
Quoted 6 lines
alloc.h new file +5
-0+extern struct alloc_state the_repository_blob_state; +extern struct alloc_state the_repository_tree_state; +extern struct alloc_state the_repository_commit_state; +extern struct alloc_state the_repository_tag_state; +extern struct alloc_state the_repository_object_state;(Context: these were in
alloc.h)These seem to be used only in
object.c, and only in one function - could we declare them "static" inside that function instead?
ok
Quoted 17 lines
Quoted 17 lines
object.c +12
-1-struct object_parser *object_parser_new (void ) +struct object_parser *object_parser_new (int is_the_repo) {struct object_parser *o =xmalloc (sizeof (*o));memset (o,0 ,sizeof (*o)); + +if (is_the_repo) { + o->blob_state = &the_repository_blob_state; + o->tree_state = &the_repository_tree_state; + o->commit_state = &the_repository_commit_state; + o->tag_state = &the_repository_tag_state; + o->object_state = &the_repository_object_state; + }else { + o->blob_state =allocate_alloc_state (); + o->tree_state =allocate_alloc_state (); + o->commit_state =allocate_alloc_state ();
Quoted 8 lines
object.c void clear_commit_marks_all(unsigned int flags) · line 474
Quoted 5 lines
object.c +3
-0+ o->tag_state =allocate_alloc_state (); + o->object_state =allocate_alloc_state (); + }return o; }I don't think saving 5 allocations is worth the code complexity (of the additional parameter).
Ok, I'll remove this overhead.
Thanks,
Stefan
Re: [PATCH 13/13] alloc: allow arbitrary repositories for alloc functions
In reply to Stefan Bellerobject.c void raw_object_store_clear(struct raw_object_store *o) · line 505
Quoted 6 lines · Stefan Beller
object.c +0
-2@@ -501,9 +516,12 @@ void raw_object_store_clear(struct raw_object_store *o)void object_parser_clear (struct object_parser *o) {/* -* TOOD free objects in o->obj_hash. -*
You need to free(o->obj_hash) too. If you just want to reuse existing obj_hash[] then at least clear it, leave no dangling pointers behind.
Quoted 9 lines
object.c +5
-0* As objects are allocated in slabs (see alloc.c), we do * not need to free each object, but each slab instead. */ +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); }
Signature · 1 line
Duy
Quoted 5 lines · Stefan Beller
I also debated if it is worth converting
alloc.cvia 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.
If I remember correctly, alloc.c was added because malloc() has too high overhead per allocation (and we create like millions of them). As long as you keep allocation overhead low, it should be ok. Note that we allocate a lot more than the mem-pool's main target (cache entries if I remember correctly). We may have a couple thousands cache entries. We already deal with a couple million of struct object.
Signature · 1 line
Duy
Quoted 17 lines
-----Original Message-----
From: Duy Nguyen <[email protected]>
Sent: Wednesday, May 2, 2018 1:02 PM
To: Stefan Beller <[email protected]>
Cc: Git Mailing List <[email protected]>; Jameson Miller <[email protected]>
Subject: Re: [PATCH 00/13] object store: allocQuoted 5 lines · Stefan Beller
I also debated if it is worth converting
alloc.cvia 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.
Quoted 6 lines
If I remember correctly,
alloc.cwas added becausemalloc()has too high overhead per allocation (and we create like millions of them). As long as you keep allocation overhead low, it should be ok. Note that we allocate a lot more than the mem-pool's main target (cache entries if I remember correctly). We may have a couple thousands cache entries. We already deal with a couple million of struct object.
The work to move cache entry allocation onto a memory pool was motivated by the fact that we are looking at indexes with millions of entries. If there is scaling concern with the current version of mem-pool, we would like to address it there as well. Or if there is improvements that can be shared, that would be nice as well.
Quoted 2 lines
--
Duy
Quoted 24 lines · Jameson Miller
Quoted 17 lines
-----Original Message-----
From: Duy Nguyen <[email protected]>
Sent: Wednesday, May 2, 2018 1:02 PM
To: Stefan Beller <[email protected]>
Cc: Git Mailing List <[email protected]>; Jameson Miller <[email protected]>
Subject: Re: [PATCH 00/13] object store: allocQuoted 5 lines · Stefan Beller
I also debated if it is worth converting
alloc.cvia 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.
Quoted 6 lines
If I remember correctly,
alloc.cwas added becausemalloc()has too high overhead per allocation (and we create like millions of them). As long as you keep allocation overhead low, it should be ok. Note that we allocate a lot more than the mem-pool's main target (cache entries if I remember correctly). We may have a couple thousands cache entries. We already deal with a couple million of struct object.
Quoted 4 lines
The work to move cache entry allocation onto a memory pool was motivated by the fact that we are looking at indexes with millions of entries. If there is scaling concern with the current version of mem-pool, we would like to address it there as well. Or if there is improvements that can be shared, that would be nice as well.
I think the two have quite different characteristics. alloc.c code is driven by overhead. struct blob is only 24 bytes each and about 1/3 the repo is blobs, and each malloc has 16 bytes overhead or so if I remember correctly. struct cache_entry at minimum in 88 bytes so relative overhead is not that a big deal (but sure reducing it is still very nice).
mem-pool is about allocation speed, but I think that's not a concern for alloc.c because when we do full rev walk, I think I/O is always the bottleneck (maybe object lookup as well). I don't see a good way to have the one memory allocator that satisfyies both to be honest. If you could allocate fixed-size cache entries most of the time (e.g. larger entries will be allocated using malloc or something, and should be a small number), then perhaps we can unify. Or if mem-pool can have an option to allocated fixed size pieces with no overhead, that would be great (sorry I still have not read that mem-pool series yet)
Signature · 1 line
Duy
Quoted 49 lines
-----Original Message-----
From: Duy Nguyen <[email protected]>
Sent: Wednesday, May 2, 2018 2:23 PM
To: Jameson Miller <[email protected]>
Cc: Stefan Beller <[email protected]>; Git Mailing List <[email protected]>
Subject: Re: [PATCH 00/13] object store: allocQuoted 11 lines · Jameson Miller
Quoted 11 lines
-----Original Message-----
From: Duy Nguyen <[email protected]>
Sent: Wednesday, May 2, 2018 1:02 PM
To: Stefan Beller <[email protected]>
Cc: Git Mailing List <[email protected]>; Jameson Miller <[email protected]>
Subject: Re: [PATCH 00/13] object store: allocOn Tue, May 1, 2018 at 11:33 PM, Stefan Beller <[email protected]>
Quoted 2 lines · wrote:
Quoted 2 lines
Quoted 2 lines
I also debated if it is worth converting
alloc.cvia this patch series or if it might make more sense to use the new mem-pool byJameson[1].
Quoted 11 lines
Quoted 10 lines
Quoted 3 lines
I vaguely wonder about the performance impact, as the object allocation code seemed to be relevant in the past.
If I remember correctly,
alloc.cwas added becausemalloc()has too high overhead per allocation (and we create like millions of them). As long as you keep allocation overhead low, it should be ok. Note that we allocate a lot more than the mem-pool's main target (cache entries if I remember correctly). We may have a couple thousands cache entries. We already deal with a couple million of struct object.Quoted 4 lines
The work to move cache entry allocation onto a memory pool was motivated by the fact that we are looking at indexes with millions of entries. If there is scaling concern with the current version of mem-pool, we would like to address it there as well. Or if there is
improvements that can be shared, that would be nice as well.
I think the two have quite different characteristics.
alloc.ccode is driven by overhead. struct blob is only 24 bytes each and about 1/3 the repo is blobs, and each malloc has 16 bytes overhead or so if I remember correctly. structcache_entryat minimum in 88 bytes so relative overhead is not that a big deal (but sure reducing it is still very nice).
Quoted 11 lines
mem-pool is about allocation speed, but I think that's not a concern for
alloc.cbecause when we do full rev walk, I think I/O is always the bottleneck (maybe object lookup as well). I don't see a good way to have the one memory allocator that satisfyies both to be honest. If you could allocate fixed-size cache entries most of the time (e.g.
larger entries will be allocated using malloc or something, and should be a small number), then perhaps we can unify. Or if mem-pool can have an option to allocated fixed size pieces with no overhead, that would be great (sorry I still have not read that mem-pool series yet)
--
Duy
Thank you for the extra details - the extra context was helpful - especially the motivations for each of the areas. I agree with your general analysis, but with the extra point that the memory pool does allocate memory (variable sized) without any overhead, except for possible alignment considerations and differences in bookkeeping the larger "blocks" of memory from which small allocations are made from - but I don't think this would be enough to have a meaningful overall impact.
The mem-pool only tracks the pointer to the next available bit of memory, and the end of the available memory range. It has a similar constraint in that individual allocations cannot be freed
- you have to free the whole block.
It may be that the requirements are different enough (or the gains worth it) to have another dedicated pooling allocator, but I think the current design of the memory pool would satisfy both consumers, even if the memory considerations are a bigger motivation for blob structs. I would be interested in your thoughts if you get the opportunity to read the mem-pool series.
Quoted 8 lines · Duy Nguyen
I think the two have quite different characteristics.
alloc.ccode is driven by overhead. struct blob is only 24 bytes each and about 1/3 the repo is blobs, and each malloc has 16 bytes overhead or so if I remember correctly. structcache_entryat minimum in 88 bytes so relative overhead is not that a big deal (but sure reducing it is still very nice).mem-pool is about allocation speed,
I don't think so, given that we do a linear search in each block allocation.
Quoted 4 lines
but I think that's not a concern
foralloc.cbecause when we do full rev walk, I think I/O is always the bottleneck (maybe object lookup as well). I don't see a good way to have the one memory allocator that satisfyies both to be honest.
By changing the allocation size of a block to be larger than 1024 entries in alloc. we should lessen the impact of management overhead, and then the mem pool can be more than feasible.
Quoted 4 lines · Stefan Beller
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.
I am not sure how well this topic is done, but I've queued the following patch at the tip of the topic to make it compile after getting merged to integration branches (curiously, the topic by itself compiled file for whatever reason). I think I haven't send that fixup patch out, so here it is.
-- >8 --
From: Junio C Hamano <[email protected]>
Date: Wed, 2 May 2018 19:09:50 +0900
Subject: [PATCH] alloc.c: include alloc.h
Otherwise the definition in alloc.c would not see the matching decl in alloc.h, triggering warning from compiler.
--- alloc.c | 1 + 1 file changed, 1 insertion(+)
alloc.c +1 -0
@@ -16,6 +16,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #define BLOCKING 1024
Signature · 1 line
2.17.0-391-g1f1cddd558
Hi Junio,
Quoted 11 lines · Junio C Hamano
Quoted 4 lines · Stefan Beller
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.
I am not sure how well this topic is done, but I've queued the following patch at the tip of the topic to make it compile after getting merged to integration branches (curiously, the topic by itself compiled file for whatever reason).
Thanks for the fixup; I will include it with other fixes in a reroll.
The investigation why it would not compile is not found in alloc.c but in 1da1580e4c2 (Makefile: detect compiler and enable more warnings in DEVELOPER=1, 2018-04-14), which enabled -Werror=missing-prototypes, that requires a prototype which is found in alloc.h
Thanks,
Stefan
v2 cover object store: alloc
In reply to Stefan Bellerv2:
* 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 protected]/
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 | 63 +++++++++++++++++----------- alloc.h | 15 +++++++ blame.c | 3 +- blob.c | 5 ++- cache.h | 9 ---- commit.c | 4 +- merge-recursive.c | 3 +- object.c | 105 +++++++++++++++++++++++++++++++++------------- object.h | 18 +++++++- repository.c | 7 ++++ repository.h | 13 +++++- tag.c | 4 +- tree.c | 4 +- 13 files changed, 184 insertions(+), 69 deletions(-) create mode 100644 alloc.h
1: 94a4aa2a825 ! 1: c40aae31a1e repository: introduce object parser field
@@ -1,23 +1,20 @@
Author: Stefan Beller <[email protected]>
- repository: introduce object parser field
+ repository: introduce parsed objects field
- Git's object access code can be thought of as containing two layers:
- the raw object store provides access to raw object content, while the
- higher level obj_hash code parses raw objects and keeps track of
- parenthood and other object relationships using 'struct object'.
- Keeping these layers separate should make it easier to find relevant
- functions and to change the implementation of one without having to
- touch the other.
+ Convert the existing global cache for parsed objects (obj_hash) into
+ repository-specific parsed object caches. Existing code that uses
+ obj_hash are modified to use the parsed object cache of
+ the_repository; future patches will use the parsed object caches of
+ other repositories.
- Add an object_parser field to 'struct repository' to prepare obj_hash
- to be handled per repository. Callers still only use the_repository
- for now --- later patches will adapt them to handle arbitrary
- repositories.
+ Another future use case for a pool of objects is ease of memory management
+ in revision walking: If we can free the rev-list related memory early in
+ pack-objects (e.g. part of repack operation) then it could lower memory
+ pressure significantly when running on large repos. While this has been
+ discussed on the mailing list lately, this series doesn't implement this.
- Signed-off-by: Jonathan Nieder <[email protected]>
Signed-off-by: Stefan Beller <[email protected]>
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/object.c b/object.c
--- a/object.c
@@ -139,9 +136,9 @@
}
}
-+struct object_parser *object_parser_new(void)
++struct parsed_object_pool *parsed_object_pool_new(void)
+{
-+ struct object_parser *o = xmalloc(sizeof(*o));
++ struct parsed_object_pool *o = xmalloc(sizeof(*o));
+ memset(o, 0, sizeof(*o));
+ return o;
+}
@@ -154,7 +151,7 @@
o->packed_git = NULL;
}
+
-+void object_parser_clear(struct object_parser *o)
++void parsed_object_pool_clear(struct parsed_object_pool *o)
+{
+ /*
+ * TOOD free objects in o->obj_hash.
@@ -171,13 +168,13 @@
#ifndef OBJECT_H
#define OBJECT_H
-+struct object_parser {
++struct parsed_object_pool {
+ struct object **obj_hash;
+ int nr_objs, obj_hash_size;
+};
+
-+struct object_parser *object_parser_new(void);
-+void object_parser_clear(struct object_parser *o);
++struct parsed_object_pool *parsed_object_pool_new(void);
++void parsed_object_pool_clear(struct parsed_object_pool *o);
+
struct object_list {
struct object *item;
@@ -198,7 +195,7 @@
the_repo.index = &the_index;
the_repo.objects = raw_object_store_new();
-+ the_repo.parsed_objects = object_parser_new();
++ the_repo.parsed_objects = parsed_object_pool_new();
+
repo_set_hash_algo(&the_repo, GIT_HASH_SHA1);
}
@@ -207,7 +204,7 @@
memset(repo, 0, sizeof(*repo));
repo->objects = raw_object_store_new();
-+ repo->parsed_objects = object_parser_new();
++ repo->parsed_objects = parsed_object_pool_new();
if (repo_init_gitdir(repo, gitdir))
goto error;
@@ -215,7 +212,7 @@
raw_object_store_clear(repo->objects);
FREE_AND_NULL(repo->objects);
-+ object_parser_clear(repo->parsed_objects);
++ parsed_object_pool_clear(repo->parsed_objects);
+ FREE_AND_NULL(repo->parsed_objects);
+
if (repo->config) {
@@ -237,11 +234,13 @@
struct raw_object_store *objects;
+ /*
-+ * State for the object parser. This owns all parsed objects
-+ * (struct object) so callers do not have to manage their
-+ * lifetime.
++ * All objects in this repository that have been parsed. This structure
++ * owns all objects it references, so users of "struct object *"
++ * generally do not need to free them; instead, when a repository is no
++ * longer used, call parsed_object_pool_clear() on this structure, which
++ * is called by the repositories repo_clear on its desconstruction.
+ */
-+ struct object_parser *parsed_objects;
++ struct parsed_object_pool *parsed_objects;
+
/* The store in which the refs are held. */
struct ref_store *refs;2: 9ddead5bb7e ! 2: 4ce05bb8b04 object: add repository argument to create_object
@@ -7,14 +7,8 @@
mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
- As with the previous commits, use a macro to catch callers passing a
- repository other than the_repository at compile time.
-
- Add the cocci patch that converted the callers.
-
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/blob.c b/blob.c
--- a/blob.c
3: 5ec44fe74de ! 3: 27e1d2621c4 object: add repository argument to grow_object_hash
@@ -7,12 +7,8 @@
mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
- As with the previous commits, use a macro to catch callers passing a
- repository other than the_repository at compile time.
-
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/object.c b/object.c
--- a/object.c
4: 2a1d0220eb9 ! 4: f8c7fc9b26f alloc: add repository argument to alloc_blob_node
@@ -2,8 +2,12 @@
alloc: add repository argument to alloc_blob_node
+ This is a small mechanical change; it doesn't change the
+ implementation to handle repositories other than the_repository yet.
+ Use a macro to catch callers passing a repository other than
+ the_repository at compile time.
+
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
5: 286cde18ff2 ! 5: b682e50bd80 alloc: add repository argument to alloc_tree_node
@@ -2,8 +2,12 @@
alloc: add repository argument to alloc_tree_node
+ This is a small mechanical change; it doesn't change the
+ implementation to handle repositories other than the_repository yet.
+ Use a macro to catch callers passing a repository other than
+ the_repository at compile time.
+
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
6: b05f0588f37 ! 6: c89a3f0ca8b alloc: add repository argument to alloc_commit_node
@@ -2,8 +2,12 @@
alloc: add repository argument to alloc_commit_node
+ This is a small mechanical change; it doesn't change the
+ implementation to handle repositories other than the_repository yet.
+ Use a macro to catch callers passing a repository other than
+ the_repository at compile time.
+
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
7: e8e25cf61fc ! 7: 59e74c6ef43 alloc: add repository argument to alloc_tag_node
@@ -2,8 +2,12 @@
alloc: add repository argument to alloc_tag_node
+ This is a small mechanical change; it doesn't change the
+ implementation to handle repositories other than the_repository yet.
+ Use a macro to catch callers passing a repository other than
+ the_repository at compile time.
+
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
8: dbd4e184121 ! 8: 6c11cf164b1 alloc: add repository argument to alloc_object_node
@@ -2,8 +2,12 @@
alloc: add repository argument to alloc_object_node
+ This is a small mechanical change; it doesn't change the
+ implementation to handle repositories other than the_repository yet.
+ Use a macro to catch callers passing a repository other than
+ the_repository at compile time.
+
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
9: ca4a23489b7 ! 9: eb824871f17 alloc: add repository argument to alloc_report
@@ -2,8 +2,12 @@
alloc: add repository argument to alloc_report
+ This is a small mechanical change; it doesn't change the
+ implementation to handle repositories other than the_repository yet.
+ Use a macro to catch callers passing a repository other than
+ the_repository at compile time.
+
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
10: f0f84627ce8 ! 10: 76e214a2254 alloc: add repository argument to alloc_commit_index
@@ -2,8 +2,12 @@
alloc: add repository argument to alloc_commit_index
+ This is a small mechanical change; it doesn't change the
+ implementation to handle repositories other than the_repository yet.
+ Use a macro to catch callers passing a repository other than
+ the_repository at compile time.
+
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
11: 60f4e269bb8 ! 11: 3a13500d480 object: allow grow_object_hash to handle arbitrary repositories
@@ -2,9 +2,9 @@
object: allow grow_object_hash to handle arbitrary repositories
+ Reviewed-by: Jonathan Tan <[email protected]>
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/object.c b/object.c
--- a/object.c
12: 5c0b35cfabf ! 12: 8e06fb06d60 object: allow create_object to handle arbitrary repositories
@@ -2,9 +2,9 @@
object: allow create_object to handle arbitrary repositories
+ Reviewed-by: Jonathan Tan <[email protected]>
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/object.c b/object.c
--- a/object.c
13: 10de7382d15 ! 13: d15807445ad alloc: allow arbitrary repositories for alloc functions
@@ -12,7 +12,6 @@
the_repository.
Signed-off-by: Stefan Beller <[email protected]>
- Signed-off-by: Junio C Hamano <[email protected]>
diff --git a/alloc.c b/alloc.c
--- a/alloc.c
@@ -26,11 +25,15 @@
+ * it maintains all the allocation infrastructure, but even more because it ends
* up with maximal alignment because it doesn't know what the object alignment
* for the new allocation is.
-+ *
-+ * TODO: Combine this with mem-pool?
*/
- #include "cache.h"
- #include "object.h"
+@@
+ #include "tree.h"
+ #include "commit.h"
+ #include "tag.h"
++#include "alloc.h"
+
+ #define BLOCKING 1024
+
@@
int count; /* total number of nodes allocated */
int nr; /* number of nodes left in current allocation */
@@ -58,19 +61,24 @@
{
void *ret;
@@
+ if (!s->nr) {
+ s->nr = BLOCKING;
+ s->p = xmalloc(BLOCKING * node_size);
++
++ ALLOC_GROW(s->slabs, s->slab_nr + 1, s->slab_alloc);
++ s->slabs[s->slab_nr++] = s->p;
+ }
+ s->nr--;
+ s->count++;
ret = s->p;
s->p = (char *)s->p + node_size;
memset(ret, 0, node_size);
-+
-+ ALLOC_GROW(s->slabs, s->slab_nr + 1, s->slab_alloc);
-+ s->slabs[s->slab_nr++] = ret;
+
return ret;
}
-static struct alloc_state blob_state;
-void *alloc_blob_node_the_repository(void)
-+struct alloc_state the_repository_blob_state;
+void *alloc_blob_node(struct repository *r)
{
- struct blob *b = alloc_node(&blob_state, sizeof(struct blob));
@@ -81,7 +89,6 @@
-static struct alloc_state tree_state;
-void *alloc_tree_node_the_repository(void)
-+struct alloc_state the_repository_tree_state;
+void *alloc_tree_node(struct repository *r)
{
- struct tree *t = alloc_node(&tree_state, sizeof(struct tree));
@@ -92,7 +99,6 @@
-static struct alloc_state tag_state;
-void *alloc_tag_node_the_repository(void)
-+struct alloc_state the_repository_tag_state;
+void *alloc_tag_node(struct repository *r)
{
- struct tag *t = alloc_node(&tag_state, sizeof(struct tag));
@@ -103,7 +109,6 @@
-static struct alloc_state object_state;
-void *alloc_object_node_the_repository(void)
-+struct alloc_state the_repository_object_state;
+void *alloc_object_node(struct repository *r)
{
- struct object *obj = alloc_node(&object_state, sizeof(union any_object));
@@ -123,7 +128,6 @@
}
-void *alloc_commit_node_the_repository(void)
-+struct alloc_state the_repository_commit_state;
+void *alloc_commit_node(struct repository *r)
{
- struct commit *c = alloc_node(&commit_state, sizeof(struct commit));
@@ -167,12 +171,6 @@
+void *allocate_alloc_state(void);
+void clear_alloc_state(struct alloc_state *s);
+
-+extern struct alloc_state the_repository_blob_state;
-+extern struct alloc_state the_repository_tree_state;
-+extern struct alloc_state the_repository_commit_state;
-+extern struct alloc_state the_repository_tag_state;
-+extern struct alloc_state the_repository_object_state;
-+
+#endif
diff --git a/blame.c b/blame.c
@@ -262,40 +260,47 @@
#include "object-store.h"
#include "packfile.h"
@@
- }
- }
-
--struct object_parser *object_parser_new(void)
-+struct object_parser *object_parser_new(int is_the_repo)
{
- struct object_parser *o = xmalloc(sizeof(*o));
+ struct parsed_object_pool *o = xmalloc(sizeof(*o));
memset(o, 0, sizeof(*o));
+
-+ if (is_the_repo) {
-+ o->blob_state = &the_repository_blob_state;
-+ o->tree_state = &the_repository_tree_state;
-+ o->commit_state = &the_repository_commit_state;
-+ o->tag_state = &the_repository_tag_state;
-+ o->object_state = &the_repository_object_state;
-+ } else {
-+ 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_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();
++
return o;
}
@@
- void object_parser_clear(struct object_parser *o)
+ void parsed_object_pool_clear(struct parsed_object_pool *o)
{
/*
- * TOOD free objects in o->obj_hash.
- *
* As objects are allocated in slabs (see alloc.c), we do
* not need to free each object, but each slab instead.
++ *
++ * Before doing so, we need to free any additional memory
++ * the objects may hold.
*/
++ unsigned i;
++
++ for (i = 0; i < o->obj_hash_size; i++) {
++ struct object *obj = o->obj_hash[i];
++
++ 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);
++ }
++ }
++
+ clear_alloc_state(o->blob_state);
+ clear_alloc_state(o->tree_state);
+ clear_alloc_state(o->commit_state);
@@ -307,7 +312,7 @@
--- a/object.h
+++ b/object.h
@@
- struct object_parser {
+ struct parsed_object_pool {
struct object **obj_hash;
int nr_objs, obj_hash_size;
+
@@ -320,33 +325,7 @@
+ unsigned commit_count;
};
--struct object_parser *object_parser_new(void);
-+struct object_parser *object_parser_new(int is_the_repo);
- void object_parser_clear(struct object_parser *o);
-
- struct object_list {
-
-diff --git a/repository.c b/repository.c
---- a/repository.c
-+++ b/repository.c
-@@
-
- the_repo.index = &the_index;
- the_repo.objects = raw_object_store_new();
-- the_repo.parsed_objects = object_parser_new();
-+ the_repo.parsed_objects = object_parser_new(1);
-
- repo_set_hash_algo(&the_repo, GIT_HASH_SHA1);
- }
-@@
- memset(repo, 0, sizeof(*repo));
-
- repo->objects = raw_object_store_new();
-- repo->parsed_objects = object_parser_new();
-+ repo->parsed_objects = object_parser_new(0);
-
- if (repo_init_gitdir(repo, gitdir))
- goto error;
+ struct parsed_object_pool *parsed_object_pool_new(void);
diff --git a/tag.c b/tag.c
--- a/tag.c14: 12e8de9e65c < -: ----------- alloc.c: include alloc.h
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 1/13 repository: introduce parsed objects field
Convert the existing global cache for parsed objects (obj_hash) into repository-specific parsed object caches. Existing code that uses obj_hash are modified to use the parsed object cache of the_repository; future patches will use the parsed object caches of other repositories.
Another future use case for a pool of objects is ease of memory management in revision walking: If we can free the rev-list related memory early in pack-objects (e.g. part of repack operation) then it could lower memory pressure significantly when running on large repos. While this has been discussed on the mailing list lately, this series doesn't implement this.
--- object.c | 63 +++++++++++++++++++++++++++++++++------------------- object.h | 8 +++++++ repository.c | 7 ++++++ repository.h | 13 ++++++++++- 4 files changed, 67 insertions(+), 24 deletions(-)
object.c +40 -23
@@ -8,17 +8,14 @@#include "object-store.h" #include "packfile.h" -static struct object **obj_hash; -static int nr_objs, obj_hash_size; -unsigned int get_max_object_index (void ) { -return obj_hash_size; +return the_repository->parsed_objects->obj_hash_size; }struct object *get_indexed_object (unsigned int idx) { -return obj_hash[idx]; +return the_repository->parsed_objects->obj_hash[idx]; }static const char *object_type_strings[] = {
@@ -90,15 +87,16 @@ struct object *lookup_object(const unsigned char *sha1)unsigned int i, first;struct object *obj; -if (!obj_hash) +if (!the_repository->parsed_objects->obj_hash)return NULL ; - first = i =hash_obj (sha1, obj_hash_size); -while ((obj = obj_hash[i]) !=NULL ) { + first = i =hash_obj (sha1, + the_repository->parsed_objects->obj_hash_size); +while ((obj = the_repository->parsed_objects->obj_hash[i]) !=NULL ) {if (!hashcmp (sha1, obj->oid.hash))break ; i++; -if (i == obj_hash_size) +if (i == the_repository->parsed_objects->obj_hash_size) i =0 ; }if (obj && i != first) {
@@ -107,7 +105,8 @@ struct object *lookup_object(const unsigned char *sha1)* that we do not need to walk the hash table the next * time we look for it. */ -SWAP (obj_hash[i], obj_hash[first]); +SWAP (the_repository->parsed_objects->obj_hash[i], + the_repository->parsed_objects->obj_hash[first]); }return obj; }
@@ -124,19 +123,19 @@ static void grow_object_hash(void)* Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = obj_hash_size <32 ?32 :2 * obj_hash_size; +int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (obj_hash); - obj_hash = new_hash; - obj_hash_size = new_hash_size; +free (the_repository->parsed_objects->obj_hash); + the_repository->parsed_objects->obj_hash = new_hash; + the_repository->parsed_objects->obj_hash_size = new_hash_size; }void *create_object (const unsigned char *sha1,void *o)
@@ -147,11 +146,12 @@ void *create_object(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (obj_hash_size -1 <= nr_objs *2 ) +if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 )grow_object_hash (); -insert_obj_hash (obj, obj_hash, obj_hash_size); - nr_objs++; +insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, + the_repository->parsed_objects->obj_hash_size); + the_repository->parsed_objects->nr_objs++;return obj; }
@@ -431,8 +431,8 @@ void clear_object_flags(unsigned flags) {int i; -for (i=0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i=0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj) obj->flags &= ~flags; }
@@ -442,13 +442,20 @@ void clear_commit_marks_all(unsigned int flags) {int i; -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj && obj->type ==OBJ_COMMIT ) obj->flags &= ~flags; } } +struct parsed_object_pool *parsed_object_pool_new (void ) +{ +struct parsed_object_pool *o =xmalloc (sizeof (*o)); +memset (o,0 ,sizeof (*o)); +return o; +} +struct raw_object_store *raw_object_store_new (void ) {struct raw_object_store *o =xmalloc (sizeof (*o));
@@ -488,3 +495,13 @@ void raw_object_store_clear(struct raw_object_store *o)close_all_packs (o); o->packed_git =NULL ; } + +void parsed_object_pool_clear (struct parsed_object_pool *o) +{ +/* +* TOOD free objects in o->obj_hash. +* +* As objects are allocated in slabs (see alloc.c), we do +* not need to free each object, but each slab instead. +*/ +}
object.h +8 -0
@@ -1,6 +1,14 @@ #ifndef OBJECT_H #define OBJECT_H +struct parsed_object_pool { + struct object **obj_hash; + int nr_objs, obj_hash_size; +}; + +struct parsed_object_pool *parsed_object_pool_new(void); +void parsed_object_pool_clear(struct parsed_object_pool *o); + struct object_list { struct object *item; struct object_list *next;
repository.c +7 -0
@@ -2,6 +2,7 @@#include "repository.h" #include "object-store.h" #include "config.h" +#include "object.h" #include "submodule-config.h" /* The main repository */
@@ -14,6 +15,8 @@ void initialize_the_repository(void) the_repo.index = &the_index; the_repo.objects =raw_object_store_new (); + the_repo.parsed_objects =parsed_object_pool_new (); +repo_set_hash_algo (&the_repo,GIT_HASH_SHA1 ); }
@@ -143,6 +146,7 @@ static int repo_init(struct repository *repo,memset (repo,0 ,sizeof (*repo)); repo->objects =raw_object_store_new (); + repo->parsed_objects =parsed_object_pool_new ();if (repo_init_gitdir (repo, gitdir)) goto error;
@@ -226,6 +230,9 @@ void repo_clear(struct repository *repo)raw_object_store_clear (repo->objects);FREE_AND_NULL (repo->objects); +parsed_object_pool_clear (repo->parsed_objects); +FREE_AND_NULL (repo->parsed_objects); +if (repo->config) {git_configset_clear (repo->config);FREE_AND_NULL (repo->config);
repository.h +12 -1
@@ -22,10 +22,21 @@ struct repository {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; +/* +* All objects in this repository that have been parsed. This structure +* owns all objects it references, so users of "struct object *" +* generally do not need to free them; instead, when a repository is no +* longer used, call parsed_object_pool_clear() on this structure, which +* is called by the repositories repo_clear on its desconstruction. +*/ +struct parsed_object_pool *parsed_objects; +/* The store in which the refs are held. */ struct ref_store *refs;
Signature · 1 line
2.17.0.255.g8bfb7c0704
Re: [PATCH v2 01/13] repository: introduce parsed objects field
repository.h struct repository { · line 27
Quoted 5 lines · On Mon, 7 May 2018 15:59:04 -0700 Stefan Beller <[email protected]> wrote:
repository.h +3
-1/* -* 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.
Update this comment - there is no more object_parser. (Maybe just delete the last sentence, or specifically name some of the functions that access this field.)
repository.h struct repository { · line 29
Quoted 2 lines
repository.h +0
-0*/ struct raw_object_store *objects;
Other than that, this patch looks good.
PATCH v2 2/13 object: add repository argument to create_object
In reply to Stefan BellerAdd a repository argument to allow the callers of create_object to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
--- blob.c | 4 +++- commit.c | 3 ++- object.c | 5 +++-- object.h | 3 ++- tag.c | 3 ++- tree.c | 3 ++- 6 files changed, 14 insertions(+), 7 deletions(-)
blob.c +3 -1
@@ -1,5 +1,6 @@#include "cache.h" #include "blob.h" +#include "repository.h" const char *blob_type ="blob" ;
@@ -7,7 +8,8 @@ struct blob *lookup_blob(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_blob_node ()); +return create_object (the_repository, oid->hash, +alloc_blob_node ());return object_as_type (obj,OBJ_BLOB ,0 ); }
commit.c +2 -1
@@ -50,7 +50,8 @@ struct commit *lookup_commit(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_commit_node ()); +return create_object (the_repository, oid->hash, +alloc_commit_node ());return object_as_type (obj,OBJ_COMMIT ,0 ); }
object.c +3 -2
@@ -138,7 +138,7 @@ static void grow_object_hash(void) the_repository->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object (const unsigned char *sha1,void *o) +void *create_object_the_repository (const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -178,7 +178,8 @@ struct object *lookup_unknown_object(const unsigned char *sha1) {struct object *obj =lookup_object (sha1);if (!obj) - obj =create_object (sha1,alloc_object_node ()); + obj =create_object (the_repository, sha1, +alloc_object_node ());return obj; }
object.h +2 -1
@@ -93,7 +93,8 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -extern void *create_object (const unsigned char *sha1,void *obj); +#define create_object (r, s, o) create_object_##r(s, o) +extern void *create_object_the_repository (const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
tag.c +2 -1
@@ -93,7 +93,8 @@ struct tag *lookup_tag(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tag_node ()); +return create_object (the_repository, oid->hash, +alloc_tag_node ());return object_as_type (obj,OBJ_TAG ,0 ); }
tree.c +2 -1
@@ -196,7 +196,8 @@ struct tree *lookup_tree(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tree_node ()); +return create_object (the_repository, oid->hash, +alloc_tree_node ());return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 3/13 object: add repository argument to grow_object_hash
In reply to Stefan BellerFrom: Jonathan Nieder <[email protected]>
Add a repository argument to allow the caller of grow_object_hash to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
--- object.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
object.c +3 -2
@@ -116,7 +116,8 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -static void grow_object_hash (void ) +#define grow_object_hash (r) grow_object_hash_##r() +static void grow_object_hash_the_repository (void ) {int i;/*
@@ -147,7 +148,7 @@ void *create_object_the_repository(const unsigned char *sha1, void *o)hashcpy (obj->oid.hash, sha1);if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (); +grow_object_hash (the_repository);insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, the_repository->parsed_objects->obj_hash_size);
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 4/13 alloc: add repository argument to alloc_blob_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- blob.c | 2 +- cache.h | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -49,7 +49,7 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size) }static struct alloc_state blob_state; -void *alloc_blob_node (void ) +void *alloc_blob_node_the_repository (void ) {struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;
blob.c +1 -1
@@ -9,7 +9,7 @@ struct blob *lookup_blob(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_blob_node ()); +alloc_blob_node (the_repository));return object_as_type (obj,OBJ_BLOB ,0 ); }
cache.h +2 -1
@@ -1764,7 +1764,8 @@ int decode_85(char *dst, const char *line, int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes);/* alloc.c */ -extern void *alloc_blob_node (void ); +#define alloc_blob_node (r) alloc_blob_node_##r() +extern void *alloc_blob_node_the_repository (void );extern void *alloc_tree_node (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 5/13 alloc: add repository argument to alloc_tree_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- tree.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -57,7 +57,7 @@ void *alloc_blob_node_the_repository(void) }static struct alloc_state tree_state; -void *alloc_tree_node (void ) +void *alloc_tree_node_the_repository (void ) {struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;
cache.h +2 -1
@@ -1766,7 +1766,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);/* alloc.c */ #define alloc_blob_node (r) alloc_blob_node_##r()extern void *alloc_blob_node_the_repository (void ); -extern void *alloc_tree_node (void ); +#define alloc_tree_node (r) alloc_tree_node_##r() +extern void *alloc_tree_node_the_repository (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );
tree.c +1 -1
@@ -197,7 +197,7 @@ struct tree *lookup_tree(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tree_node ()); +alloc_tree_node (the_repository));return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 6/13 alloc: add repository argument to alloc_commit_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- blame.c | 2 +- cache.h | 3 ++- commit.c | 2 +- merge-recursive.c | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-)
alloc.c +1 -1
@@ -88,7 +88,7 @@ unsigned int alloc_commit_index(void)return count++; } -void *alloc_commit_node (void ) +void *alloc_commit_node_the_repository (void ) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ;
blame.c +1 -1
@@ -161,7 +161,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,read_cache ();time (&now); - commit =alloc_commit_node (); + commit =alloc_commit_node (the_repository); commit->object.parsed =1 ; commit->date = now; parent_tail = &commit->parents;
cache.h +2 -1
@@ -1768,7 +1768,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);extern void *alloc_blob_node_the_repository (void );#define alloc_tree_node (r) alloc_tree_node_##r()extern void *alloc_tree_node_the_repository (void ); -extern void *alloc_commit_node (void ); +#define alloc_commit_node (r) alloc_commit_node_##r() +extern void *alloc_commit_node_the_repository (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );extern void alloc_report (void );
commit.c +1 -1
@@ -51,7 +51,7 @@ struct commit *lookup_commit(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_commit_node ()); +alloc_commit_node (the_repository));return object_as_type (obj,OBJ_COMMIT ,0 ); }
merge-recursive.c +1 -1
@@ -98,7 +98,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two,static struct commit *make_virtual_commit (struct tree *tree,const char *comment) { -struct commit *commit =alloc_commit_node (); +struct commit *commit =alloc_commit_node (the_repository);set_merge_remote_desc (commit, comment, (struct object *)commit); commit->tree = tree;
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 7/13 alloc: add repository argument to alloc_tag_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- tag.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -65,7 +65,7 @@ void *alloc_tree_node_the_repository(void) }static struct alloc_state tag_state; -void *alloc_tag_node (void ) +void *alloc_tag_node_the_repository (void ) {struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;
cache.h +2 -1
@@ -1770,7 +1770,8 @@ extern void *alloc_blob_node_the_repository(void);extern void *alloc_tree_node_the_repository (void );#define alloc_commit_node (r) alloc_commit_node_##r()extern void *alloc_commit_node_the_repository (void ); -extern void *alloc_tag_node (void ); +#define alloc_tag_node (r) alloc_tag_node_##r() +extern void *alloc_tag_node_the_repository (void );extern void *alloc_object_node (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
tag.c +1 -1
@@ -94,7 +94,7 @@ struct tag *lookup_tag(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tag_node ()); +alloc_tag_node (the_repository));return object_as_type (obj,OBJ_TAG ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 8/13 alloc: add repository argument to alloc_object_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- object.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -73,7 +73,7 @@ void *alloc_tag_node_the_repository(void) }static struct alloc_state object_state; -void *alloc_object_node (void ) +void *alloc_object_node_the_repository (void ) {struct object *obj =alloc_node (&object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;
cache.h +2 -1
@@ -1772,7 +1772,8 @@ extern void *alloc_tree_node_the_repository(void);extern void *alloc_commit_node_the_repository (void );#define alloc_tag_node (r) alloc_tag_node_##r()extern void *alloc_tag_node_the_repository (void ); -extern void *alloc_object_node (void ); +#define alloc_object_node (r) alloc_object_node_##r() +extern void *alloc_object_node_the_repository (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
object.c +1 -1
@@ -180,7 +180,7 @@ struct object *lookup_unknown_object(const unsigned char *sha1)struct object *obj =lookup_object (sha1);if (!obj) obj =create_object (the_repository, sha1, -alloc_object_node ()); +alloc_object_node (the_repository));return obj; }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 9/13 alloc: add repository argument to alloc_report
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
alloc.c +1 -1
@@ -105,7 +105,7 @@ static void report(const char *name, unsigned int count, size_t size)#define REPORT (name, type) \ report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) -void alloc_report (void ) +void alloc_report_the_repository (void ) { REPORT(blob, struct blob); REPORT(tree, struct tree);
cache.h +2 -1
@@ -1774,7 +1774,8 @@ extern void *alloc_commit_node_the_repository(void);extern void *alloc_tag_node_the_repository (void );#define alloc_object_node (r) alloc_object_node_##r()extern void *alloc_object_node_the_repository (void ); -extern void alloc_report (void ); +#define alloc_report (r) alloc_report_##r() +extern void alloc_report_the_repository (void );extern unsigned int alloc_commit_index (void );/* pkt-line.c */
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 10/13 alloc: add repository argument to alloc_commit_index
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 4 ++-- cache.h | 3 ++- object.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-)
alloc.c +2 -2
@@ -82,7 +82,7 @@ void *alloc_object_node_the_repository(void)static struct alloc_state commit_state; -unsigned int alloc_commit_index (void ) +unsigned int alloc_commit_index_the_repository (void ) {static unsigned int count;return count++;
@@ -92,7 +92,7 @@ void *alloc_commit_node_the_repository(void) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (); + c->index =alloc_commit_index (the_repository);return c; }
cache.h +2 -1
@@ -1776,7 +1776,8 @@ extern void *alloc_tag_node_the_repository(void);extern void *alloc_object_node_the_repository (void );#define alloc_report (r) alloc_report_##r()extern void alloc_report_the_repository (void ); -extern unsigned int alloc_commit_index (void ); +#define alloc_commit_index (r) alloc_commit_index_##r() +extern unsigned int alloc_commit_index_the_repository (void );/* pkt-line.c */ void packet_trace_identity (const char *prog);
object.c +1 -1
@@ -162,7 +162,7 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet)return obj; else if (obj->type == OBJ_NONE) {if (type ==OBJ_COMMIT ) - ((struct commit *)obj)->index =alloc_commit_index (); + ((struct commit *)obj)->index =alloc_commit_index (the_repository); obj->type = type;return obj; }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 11/13 object: allow grow_object_hash to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
object.c +8 -8
@@ -116,27 +116,27 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -#define grow_object_hash (r) grow_object_hash_##r() -static void grow_object_hash_the_repository (void ) +static void grow_object_hash (struct repository *r) {int i;/* * Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size; +int new_hash_size = r->parsed_objects->obj_hash_size <32 ?32 :2 * r->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { -struct object *obj = the_repository->parsed_objects->obj_hash[i]; +for (i =0 ; i < r->parsed_objects->obj_hash_size; i++) { +struct object *obj = r->parsed_objects->obj_hash[i]; +if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (the_repository->parsed_objects->obj_hash); - the_repository->parsed_objects->obj_hash = new_hash; - the_repository->parsed_objects->obj_hash_size = new_hash_size; +free (r->parsed_objects->obj_hash); + r->parsed_objects->obj_hash = new_hash; + r->parsed_objects->obj_hash_size = new_hash_size; }void *create_object_the_repository (const unsigned char *sha1,void *o)
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 12/13 object: allow create_object to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 12 ++++++------ object.h | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-)
object.c +6 -6
@@ -139,7 +139,7 @@ static void grow_object_hash(struct repository *r) r->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object_the_repository (const unsigned char *sha1,void *o) +void *create_object (struct repository *r,const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -147,12 +147,12 @@ void *create_object_the_repository(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (the_repository); +if (r->parsed_objects->obj_hash_size -1 <= r->parsed_objects->nr_objs *2 ) +grow_object_hash (r); -insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, - the_repository->parsed_objects->obj_hash_size); - the_repository->parsed_objects->nr_objs++; +insert_obj_hash (obj, r->parsed_objects->obj_hash, + r->parsed_objects->obj_hash_size); + r->parsed_objects->nr_objs++;return obj; }
object.h +1 -2
@@ -93,8 +93,7 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -#define create_object (r, s, o) create_object_##r(s, o) -extern void *create_object_the_repository (const unsigned char *sha1,void *obj); +extern void *create_object (struct repository *r,const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v2 13/13 alloc: allow arbitrary repositories for alloc functions
In reply to Stefan BellerWe have to convert all of the alloc functions at once, because alloc_report uses a funky macro for reporting. It is better for the sake of mechanical conversion to convert multiple functions at once rather than changing the structure of the reporting function.
We record all memory allocation in alloc.c, and free them in clear_alloc_state, which is called for all repositories except the_repository.
--- alloc.c | 63 +++++++++++++++++++++++++++++------------------ alloc.h | 15 +++++++++++ blame.c | 1 + blob.c | 1 + cache.h | 16 ------------ commit.c | 1 + merge-recursive.c | 1 + object.c | 34 +++++++++++++++++++++++-- object.h | 8 ++++++ tag.c | 1 + tree.c | 1 + 11 files changed, 100 insertions(+), 42 deletions(-) create mode 100644 alloc.h
alloc.c +39 -24
@@ -4,8 +4,7 @@* Copyright (C) 2006 Linus Torvalds * * The standard malloc/free wastes too much space for objects, partly because -* it maintains all the allocation infrastructure (which isn't needed, since -* we never free an object descriptor anyway), but even more because it ends +* it maintains all the allocation infrastructure, but even more because it ends * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. */
@@ -15,6 +14,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #define BLOCKING 1024
@@ -30,8 +30,25 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs; +int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + } +} +static inline void *alloc_node (struct alloc_state *s,size_t node_size) {void *ret;
@@ -39,60 +56,57 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)if (!s->nr) { s->nr =BLOCKING ; s->p =xmalloc (BLOCKING * node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = s->p; } s->nr--; s->count++; ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); +return ret; } -static struct alloc_state blob_state; -void *alloc_blob_node_the_repository (void ) +void *alloc_blob_node (struct repository *r) { -struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); +struct blob *b =alloc_node (r->parsed_objects->blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;return b; } -static struct alloc_state tree_state; -void *alloc_tree_node_the_repository (void ) +void *alloc_tree_node (struct repository *r) { -struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); +struct tree *t =alloc_node (r->parsed_objects->tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;return t; } -static struct alloc_state tag_state; -void *alloc_tag_node_the_repository (void ) +void *alloc_tag_node (struct repository *r) { -struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); +struct tag *t =alloc_node (r->parsed_objects->tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;return t; } -static struct alloc_state object_state; -void *alloc_object_node_the_repository (void ) +void *alloc_object_node (struct repository *r) { -struct object *obj =alloc_node (&object_state,sizeof (union any_object )); +struct object *obj =alloc_node (r->parsed_objects->object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;return obj; } -static struct alloc_state commit_state; - -unsigned int alloc_commit_index_the_repository (void ) +unsigned int alloc_commit_index (struct repository *r) { -static unsigned int count; -return count++; +return r->parsed_objects->commit_count++; } -void *alloc_commit_node_the_repository (void ) +void *alloc_commit_node (struct repository *r) { -struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); +struct commit *c =alloc_node (r->parsed_objects->commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (the_repository); + c->index =alloc_commit_index (r);return c; }
@@ -103,9 +117,10 @@ static void report(const char *name, unsigned int count, size_t size) }#define REPORT (name, type) \ - report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) + report(#name, r->parsed_objects->name##_state->count, \ + r->parsed_objects->name##_state->count * sizeof(type) >> 10) -void alloc_report_the_repository (void ) +void alloc_report (struct repository *r) { REPORT(blob, struct blob); REPORT(tree, struct tree);
alloc.h new file +15 -0
@@ -0,0 +1,15 @@ +#ifndef ALLOC_H +#define ALLOC_H + +void *alloc_blob_node (struct repository *r); +void *alloc_tree_node (struct repository *r); +void *alloc_commit_node (struct repository *r); +void *alloc_tag_node (struct repository *r); +void *alloc_object_node (struct repository *r); +void alloc_report (struct repository *r); +unsigned int alloc_commit_index (struct repository *r); + +void *allocate_alloc_state (void ); +void clear_alloc_state (struct alloc_state *s); + +#endif
blame.c +1 -0
@@ -6,6 +6,7 @@#include "diffcore.h" #include "tag.h" #include "blame.h" +#include "alloc.h" void blame_origin_decref (struct blame_origin *o) {
blob.c +1 -0
@@ -1,6 +1,7 @@#include "cache.h" #include "blob.h" #include "repository.h" +#include "alloc.h" const char *blob_type ="blob" ;
cache.h +0 -16
@@ -1763,22 +1763,6 @@ extern const char *excludes_file;int decode_85 (char *dst,const char *line,int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes); -/* alloc.c */ -#define alloc_blob_node (r) alloc_blob_node_##r() -extern void *alloc_blob_node_the_repository (void ); -#define alloc_tree_node (r) alloc_tree_node_##r() -extern void *alloc_tree_node_the_repository (void ); -#define alloc_commit_node (r) alloc_commit_node_##r() -extern void *alloc_commit_node_the_repository (void ); -#define alloc_tag_node (r) alloc_tag_node_##r() -extern void *alloc_tag_node_the_repository (void ); -#define alloc_object_node (r) alloc_object_node_##r() -extern void *alloc_object_node_the_repository (void ); -#define alloc_report (r) alloc_report_##r() -extern void alloc_report_the_repository (void ); -#define alloc_commit_index (r) alloc_commit_index_##r() -extern unsigned int alloc_commit_index_the_repository (void ); -/* pkt-line.c */ void packet_trace_identity (const char *prog);
commit.c +1 -0
@@ -6,6 +6,7 @@#include "diff.h" #include "revision.h" #include "notes.h" +#include "alloc.h" #include "gpg-interface.h" #include "mergesort.h" #include "commit-slab.h"
merge-recursive.c +1 -0
@@ -14,6 +14,7 @@#include "tree-walk.h" #include "diff.h" #include "diffcore.h" +#include "alloc.h" #include "tag.h" #include "unpack-trees.h" #include "string-list.h"
object.c +32 -2
@@ -4,6 +4,7 @@#include "blob.h" #include "tree.h" #include "commit.h" +#include "alloc.h" #include "tag.h" #include "object-store.h" #include "packfile.h"
@@ -455,6 +456,13 @@ 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 (); +return o; }
@@ -501,9 +509,31 @@ void raw_object_store_clear(struct raw_object_store *o)void parsed_object_pool_clear (struct parsed_object_pool *o) {/* -* TOOD free objects in o->obj_hash. -* * As objects are allocated in slabs (see alloc.c), we do * not need to free each object, but each slab instead. +* +* Before doing so, we need to free any additional memory +* the objects may hold. */ +unsigned i; + +for (i =0 ; i < o->obj_hash_size; i++) { +struct object *obj = o->obj_hash[i]; + +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); + } + } + +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); }
object.h +8 -0
@@ -4,6 +4,14 @@struct parsed_object_pool {struct object **obj_hash;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; +unsigned commit_count; };struct parsed_object_pool *parsed_object_pool_new (void );
tag.c +1 -0
@@ -3,6 +3,7 @@#include "commit.h" #include "tree.h" #include "blob.h" +#include "alloc.h" #include "gpg-interface.h" const char *tag_type ="tag" ;
tree.c +1 -0
@@ -5,6 +5,7 @@#include "blob.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "tree-walk.h" const char *tree_type ="tree" ;
Signature · 1 line
2.17.0.255.g8bfb7c0704
Re: [PATCH v2 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 6 lines · Stefan Beller
object.c +3
-0@@ -501,9 +509,31 @@ void raw_object_store_clear(struct raw_object_store *o)void parsed_object_pool_clear (struct parsed_object_pool *o) + for (i = 0; i < o->obj_hash_size; i++) { +struct object *obj = o->obj_hash[i]; +
object.c void raw_object_store_clear(struct raw_object_store *o) · line 532
Quoted 10 lines
object.c +10
-0+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); + } + }
Coverity complains about this final free(). I think the "&" is doing an incorrect extra level of indirection?
That said, I'm not sure if it is safe to blindly free the util field. We don't necessarily know what downstream code has pointed it to. It may not be allocated memory[1], or it may even be a more complicated data structure that has sub-components that need freeing[2].
In the long run, it may be worth trying to get rid of this util field completely, in favor of having callers use a commit_slab. That has better memory-ownership semantics, and it would save 8 bytes in struct commit.
[1] Grepping for "commit->util =", sequencer.c seems to assign pointers into other arrays, as well as the "(void *)1".
[2] Most assignments seem to be flex-structs, but blame.c assigns a linked list.
-Peff
Re: [PATCH v2 13/13] alloc: allow arbitrary repositories for alloc functions
In reply to Stefan BellerQuoted 12 lines · Stefan Beller
object.c +3
-2@@ -501,9 +509,31 @@ void raw_object_store_clear(struct raw_object_store *o)void parsed_object_pool_clear (struct parsed_object_pool *o) {/* -* TOOD free objects in o->obj_hash. -* * As objects are allocated in slabs (see alloc.c), we do * not need to free each object, but each slab instead. +* +* Before doing so, we need to free any additional memory +* the objects may hold. */
object.c void raw_object_store_clear(struct raw_object_store *o) · line 527
Quoted 10 lines
object.c +10
-0+unsigned i; + +for (i =0 ; i < o->obj_hash_size; i++) { +struct object *obj = o->obj_hash[i]; + +if (!obj) +continue ; + +if (obj->type ==OBJ_TREE ) { +free (((struct tree *)obj)->buffer);
It would be nicer to keep this in separate functions, e.g. release_tree_node() and release_commit_node() to go with alloc_xxx_node().
object.c void raw_object_store_clear(struct raw_object_store *o) · line 532
Quoted 5 lines
object.c +5
-0+ }else if (obj->type ==OBJ_COMMIT ) { +free_commit_list (((struct commit *)obj)->parents); +free (&((struct commit *)obj)->util); + } + }
I still don't see who frees obj_hash[] (or at least clears it if not freed). If I'm going to use this to free memory in pack-objects then I'd really prefer obj_hash[] freed because it's a big _big_ array.
Just to be clear, what I mean is
FREE_AND_NULL(o->obj_hash);o->obj_hash_size = 0;
Quoted 7 lines
object.c +6
-0+ +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); }
Signature · 1 line
Duy
Re: [PATCH v2 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 19 lines · Duy Nguyen
Quoted 18 lines · Stefan Beller
object.c +9
-2@@ -501,9 +509,31 @@ void raw_object_store_clear(struct raw_object_store *o)void parsed_object_pool_clear (struct parsed_object_pool *o) {/* -* TOOD free objects in o->obj_hash. -* * As objects are allocated in slabs (see alloc.c), we do * not need to free each object, but each slab instead. +* +* Before doing so, we need to free any additional memory +* the objects may hold. */ +unsigned i; + +for (i =0 ; i < o->obj_hash_size; i++) { +struct object *obj = o->obj_hash[i]; + +if (!obj)
Quoted 8 lines
object.c void raw_object_store_clear(struct raw_object_store *o) · line 527
Quoted 4 lines
object.c +4
-0+continue ; + +if (obj->type ==OBJ_TREE ) { +free (((struct tree *)obj)->buffer);It would be nicer to keep this in separate functions, e.g.
release_tree_node()andrelease_commit_node()to go withalloc_xxx_node().
ok, I can introduce that, although it seems unnecessary complicated for now.
On top of this series I started an experiment (which rewrites alloc and object.c a whole lot more; for performance reasons), which gets rid of the multiple alloc_states. There will be only one allocation for one repository, it can allocate across multiple types without alignment overhead. It will reduce memory footprint of obj_hash by half, via storing indexes instead of pointers in there.
That said, the experiment shall not influence the
direction of this series. Will fix.
Quoted 9 lines
object.c void raw_object_store_clear(struct raw_object_store *o) · line 532
Quoted 5 lines
object.c +5
-0+ }else if (obj->type ==OBJ_COMMIT ) { +free_commit_list (((struct commit *)obj)->parents); +free (&((struct commit *)obj)->util); + } + }I still don't see who frees obj_hash[] (or at least clears it if not freed). If I'm going to use this to free memory in pack-objects then I'd really prefer obj_hash[] freed because it's a big _big_ array.
gah!
Quoted 4 lines
Just to be clear, what I mean is
FREE_AND_NULL(o->obj_hash);o->obj_hash_size= 0;
ok, I just put it here, just before the calls
to clear_alloc_state()s.
Re: [PATCH v2 13/13] alloc: allow arbitrary repositories for alloc functions
In reply to Stefan Bellerobject.c void raw_object_store_clear(struct raw_object_store *o) · line 531
Quoted 12 lines · On Mon, 7 May 2018 15:59:16 -0700 Stefan Beller <[email protected]> wrote:
object.c +12
-0+for (i =0 ; i < o->obj_hash_size; i++) { +struct object *obj = o->obj_hash[i]; + +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); + }
Besides the other comments by Peff and Duy, should the "tag" field of a tag object be freed too? It is allocated by xmemdupz in tag.c, and is not assigned to by any other code (verified by renaming it and then fixing the compile errors one by one).
Other than that, and other than my small comment on patch 1, this patch set looks good to me.
v3 cover object store: alloc
In reply to Stefan Bellerv3:
* 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 protected]/
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
Signature · 139 lines
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 protected]>
- Signed-off-by: Junio C Hamano <[email protected]>
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);PATCH v3 1/13 repository: introduce parsed objects field
Convert the existing global cache for parsed objects (obj_hash) into repository-specific parsed object caches. Existing code that uses obj_hash are modified to use the parsed object cache of the_repository; future patches will use the parsed object caches of other repositories.
Another future use case for a pool of objects is ease of memory management in revision walking: If we can free the rev-list related memory early in pack-objects (e.g. part of repack operation) then it could lower memory pressure significantly when running on large repos. While this has been discussed on the mailing list lately, this series doesn't implement this.
--- object.c | 63 +++++++++++++++++++++++++++++++++------------------- object.h | 8 +++++++ repository.c | 7 ++++++ repository.h | 9 ++++++++ 4 files changed, 64 insertions(+), 23 deletions(-)
object.c +40 -23
@@ -8,17 +8,14 @@#include "object-store.h" #include "packfile.h" -static struct object **obj_hash; -static int nr_objs, obj_hash_size; -unsigned int get_max_object_index (void ) { -return obj_hash_size; +return the_repository->parsed_objects->obj_hash_size; }struct object *get_indexed_object (unsigned int idx) { -return obj_hash[idx]; +return the_repository->parsed_objects->obj_hash[idx]; }static const char *object_type_strings[] = {
@@ -90,15 +87,16 @@ struct object *lookup_object(const unsigned char *sha1)unsigned int i, first;struct object *obj; -if (!obj_hash) +if (!the_repository->parsed_objects->obj_hash)return NULL ; - first = i =hash_obj (sha1, obj_hash_size); -while ((obj = obj_hash[i]) !=NULL ) { + first = i =hash_obj (sha1, + the_repository->parsed_objects->obj_hash_size); +while ((obj = the_repository->parsed_objects->obj_hash[i]) !=NULL ) {if (!hashcmp (sha1, obj->oid.hash))break ; i++; -if (i == obj_hash_size) +if (i == the_repository->parsed_objects->obj_hash_size) i =0 ; }if (obj && i != first) {
@@ -107,7 +105,8 @@ struct object *lookup_object(const unsigned char *sha1)* that we do not need to walk the hash table the next * time we look for it. */ -SWAP (obj_hash[i], obj_hash[first]); +SWAP (the_repository->parsed_objects->obj_hash[i], + the_repository->parsed_objects->obj_hash[first]); }return obj; }
@@ -124,19 +123,19 @@ static void grow_object_hash(void)* Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = obj_hash_size <32 ?32 :2 * obj_hash_size; +int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (obj_hash); - obj_hash = new_hash; - obj_hash_size = new_hash_size; +free (the_repository->parsed_objects->obj_hash); + the_repository->parsed_objects->obj_hash = new_hash; + the_repository->parsed_objects->obj_hash_size = new_hash_size; }void *create_object (const unsigned char *sha1,void *o)
@@ -147,11 +146,12 @@ void *create_object(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (obj_hash_size -1 <= nr_objs *2 ) +if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 )grow_object_hash (); -insert_obj_hash (obj, obj_hash, obj_hash_size); - nr_objs++; +insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, + the_repository->parsed_objects->obj_hash_size); + the_repository->parsed_objects->nr_objs++;return obj; }
@@ -431,8 +431,8 @@ void clear_object_flags(unsigned flags) {int i; -for (i=0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i=0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj) obj->flags &= ~flags; }
@@ -442,13 +442,20 @@ void clear_commit_marks_all(unsigned int flags) {int i; -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj && obj->type ==OBJ_COMMIT ) obj->flags &= ~flags; } } +struct parsed_object_pool *parsed_object_pool_new (void ) +{ +struct parsed_object_pool *o =xmalloc (sizeof (*o)); +memset (o,0 ,sizeof (*o)); +return o; +} +struct raw_object_store *raw_object_store_new (void ) {struct raw_object_store *o =xmalloc (sizeof (*o));
@@ -488,3 +495,13 @@ void raw_object_store_clear(struct raw_object_store *o)close_all_packs (o); o->packed_git =NULL ; } + +void parsed_object_pool_clear (struct parsed_object_pool *o) +{ +/* +* TOOD free objects in o->obj_hash. +* +* As objects are allocated in slabs (see alloc.c), we do +* not need to free each object, but each slab instead. +*/ +}
object.h +8 -0
@@ -1,6 +1,14 @@ #ifndef OBJECT_H #define OBJECT_H +struct parsed_object_pool { + struct object **obj_hash; + int nr_objs, obj_hash_size; +}; + +struct parsed_object_pool *parsed_object_pool_new(void); +void parsed_object_pool_clear(struct parsed_object_pool *o); + struct object_list { struct object *item; struct object_list *next;
repository.c +7 -0
@@ -2,6 +2,7 @@#include "repository.h" #include "object-store.h" #include "config.h" +#include "object.h" #include "submodule-config.h" /* The main repository */
@@ -14,6 +15,8 @@ void initialize_the_repository(void) the_repo.index = &the_index; the_repo.objects =raw_object_store_new (); + the_repo.parsed_objects =parsed_object_pool_new (); +repo_set_hash_algo (&the_repo,GIT_HASH_SHA1 ); }
@@ -143,6 +146,7 @@ static int repo_init(struct repository *repo,memset (repo,0 ,sizeof (*repo)); repo->objects =raw_object_store_new (); + repo->parsed_objects =parsed_object_pool_new ();if (repo_init_gitdir (repo, gitdir)) goto error;
@@ -226,6 +230,9 @@ void repo_clear(struct repository *repo)raw_object_store_clear (repo->objects);FREE_AND_NULL (repo->objects); +parsed_object_pool_clear (repo->parsed_objects); +FREE_AND_NULL (repo->parsed_objects); +if (repo->config) {git_configset_clear (repo->config);FREE_AND_NULL (repo->config);
repository.h +9 -0
@@ -26,6 +26,15 @@ struct repository {*/ struct raw_object_store *objects; +/* +* All objects in this repository that have been parsed. This structure +* owns all objects it references, so users of "struct object *" +* generally do not need to free them; instead, when a repository is no +* longer used, call parsed_object_pool_clear() on this structure, which +* is called by the repositories repo_clear on its desconstruction. +*/ +struct parsed_object_pool *parsed_objects; +/* The store in which the refs are held. */ struct ref_store *refs;
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 2/13 object: add repository argument to create_object
In reply to Stefan BellerAdd a repository argument to allow the callers of create_object to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
--- blob.c | 4 +++- commit.c | 3 ++- object.c | 5 +++-- object.h | 3 ++- tag.c | 3 ++- tree.c | 3 ++- 6 files changed, 14 insertions(+), 7 deletions(-)
blob.c +3 -1
@@ -1,5 +1,6 @@#include "cache.h" #include "blob.h" +#include "repository.h" const char *blob_type ="blob" ;
@@ -7,7 +8,8 @@ struct blob *lookup_blob(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_blob_node ()); +return create_object (the_repository, oid->hash, +alloc_blob_node ());return object_as_type (obj,OBJ_BLOB ,0 ); }
commit.c +2 -1
@@ -50,7 +50,8 @@ struct commit *lookup_commit(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_commit_node ()); +return create_object (the_repository, oid->hash, +alloc_commit_node ());return object_as_type (obj,OBJ_COMMIT ,0 ); }
object.c +3 -2
@@ -138,7 +138,7 @@ static void grow_object_hash(void) the_repository->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object (const unsigned char *sha1,void *o) +void *create_object_the_repository (const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -178,7 +178,8 @@ struct object *lookup_unknown_object(const unsigned char *sha1) {struct object *obj =lookup_object (sha1);if (!obj) - obj =create_object (sha1,alloc_object_node ()); + obj =create_object (the_repository, sha1, +alloc_object_node ());return obj; }
object.h +2 -1
@@ -93,7 +93,8 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -extern void *create_object (const unsigned char *sha1,void *obj); +#define create_object (r, s, o) create_object_##r(s, o) +extern void *create_object_the_repository (const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
tag.c +2 -1
@@ -93,7 +93,8 @@ struct tag *lookup_tag(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tag_node ()); +return create_object (the_repository, oid->hash, +alloc_tag_node ());return object_as_type (obj,OBJ_TAG ,0 ); }
tree.c +2 -1
@@ -196,7 +196,8 @@ struct tree *lookup_tree(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tree_node ()); +return create_object (the_repository, oid->hash, +alloc_tree_node ());return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 3/13 object: add repository argument to grow_object_hash
In reply to Stefan BellerFrom: Jonathan Nieder <[email protected]>
Add a repository argument to allow the caller of grow_object_hash to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
--- object.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
object.c +3 -2
@@ -116,7 +116,8 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -static void grow_object_hash (void ) +#define grow_object_hash (r) grow_object_hash_##r() +static void grow_object_hash_the_repository (void ) {int i;/*
@@ -147,7 +148,7 @@ void *create_object_the_repository(const unsigned char *sha1, void *o)hashcpy (obj->oid.hash, sha1);if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (); +grow_object_hash (the_repository);insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, the_repository->parsed_objects->obj_hash_size);
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 4/13 alloc: add repository argument to alloc_blob_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- blob.c | 2 +- cache.h | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -49,7 +49,7 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size) }static struct alloc_state blob_state; -void *alloc_blob_node (void ) +void *alloc_blob_node_the_repository (void ) {struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;
blob.c +1 -1
@@ -9,7 +9,7 @@ struct blob *lookup_blob(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_blob_node ()); +alloc_blob_node (the_repository));return object_as_type (obj,OBJ_BLOB ,0 ); }
cache.h +2 -1
@@ -1764,7 +1764,8 @@ int decode_85(char *dst, const char *line, int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes);/* alloc.c */ -extern void *alloc_blob_node (void ); +#define alloc_blob_node (r) alloc_blob_node_##r() +extern void *alloc_blob_node_the_repository (void );extern void *alloc_tree_node (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 5/13 alloc: add repository argument to alloc_tree_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- tree.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -57,7 +57,7 @@ void *alloc_blob_node_the_repository(void) }static struct alloc_state tree_state; -void *alloc_tree_node (void ) +void *alloc_tree_node_the_repository (void ) {struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;
cache.h +2 -1
@@ -1766,7 +1766,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);/* alloc.c */ #define alloc_blob_node (r) alloc_blob_node_##r()extern void *alloc_blob_node_the_repository (void ); -extern void *alloc_tree_node (void ); +#define alloc_tree_node (r) alloc_tree_node_##r() +extern void *alloc_tree_node_the_repository (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );
tree.c +1 -1
@@ -197,7 +197,7 @@ struct tree *lookup_tree(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tree_node ()); +alloc_tree_node (the_repository));return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 6/13 alloc: add repository argument to alloc_commit_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- blame.c | 2 +- cache.h | 3 ++- commit.c | 2 +- merge-recursive.c | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-)
alloc.c +1 -1
@@ -88,7 +88,7 @@ unsigned int alloc_commit_index(void)return count++; } -void *alloc_commit_node (void ) +void *alloc_commit_node_the_repository (void ) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ;
blame.c +1 -1
@@ -161,7 +161,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,read_cache ();time (&now); - commit =alloc_commit_node (); + commit =alloc_commit_node (the_repository); commit->object.parsed =1 ; commit->date = now; parent_tail = &commit->parents;
cache.h +2 -1
@@ -1768,7 +1768,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);extern void *alloc_blob_node_the_repository (void );#define alloc_tree_node (r) alloc_tree_node_##r()extern void *alloc_tree_node_the_repository (void ); -extern void *alloc_commit_node (void ); +#define alloc_commit_node (r) alloc_commit_node_##r() +extern void *alloc_commit_node_the_repository (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );extern void alloc_report (void );
commit.c +1 -1
@@ -51,7 +51,7 @@ struct commit *lookup_commit(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_commit_node ()); +alloc_commit_node (the_repository));return object_as_type (obj,OBJ_COMMIT ,0 ); }
merge-recursive.c +1 -1
@@ -98,7 +98,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two,static struct commit *make_virtual_commit (struct tree *tree,const char *comment) { -struct commit *commit =alloc_commit_node (); +struct commit *commit =alloc_commit_node (the_repository);set_merge_remote_desc (commit, comment, (struct object *)commit); commit->tree = tree;
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 7/13 alloc: add repository argument to alloc_tag_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- tag.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -65,7 +65,7 @@ void *alloc_tree_node_the_repository(void) }static struct alloc_state tag_state; -void *alloc_tag_node (void ) +void *alloc_tag_node_the_repository (void ) {struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;
cache.h +2 -1
@@ -1770,7 +1770,8 @@ extern void *alloc_blob_node_the_repository(void);extern void *alloc_tree_node_the_repository (void );#define alloc_commit_node (r) alloc_commit_node_##r()extern void *alloc_commit_node_the_repository (void ); -extern void *alloc_tag_node (void ); +#define alloc_tag_node (r) alloc_tag_node_##r() +extern void *alloc_tag_node_the_repository (void );extern void *alloc_object_node (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
tag.c +1 -1
@@ -94,7 +94,7 @@ struct tag *lookup_tag(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tag_node ()); +alloc_tag_node (the_repository));return object_as_type (obj,OBJ_TAG ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 8/13 alloc: add repository argument to alloc_object_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- object.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -73,7 +73,7 @@ void *alloc_tag_node_the_repository(void) }static struct alloc_state object_state; -void *alloc_object_node (void ) +void *alloc_object_node_the_repository (void ) {struct object *obj =alloc_node (&object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;
cache.h +2 -1
@@ -1772,7 +1772,8 @@ extern void *alloc_tree_node_the_repository(void);extern void *alloc_commit_node_the_repository (void );#define alloc_tag_node (r) alloc_tag_node_##r()extern void *alloc_tag_node_the_repository (void ); -extern void *alloc_object_node (void ); +#define alloc_object_node (r) alloc_object_node_##r() +extern void *alloc_object_node_the_repository (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
object.c +1 -1
@@ -180,7 +180,7 @@ struct object *lookup_unknown_object(const unsigned char *sha1)struct object *obj =lookup_object (sha1);if (!obj) obj =create_object (the_repository, sha1, -alloc_object_node ()); +alloc_object_node (the_repository));return obj; }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 9/13 alloc: add repository argument to alloc_report
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
alloc.c +1 -1
@@ -105,7 +105,7 @@ static void report(const char *name, unsigned int count, size_t size)#define REPORT (name, type) \ report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) -void alloc_report (void ) +void alloc_report_the_repository (void ) { REPORT(blob, struct blob); REPORT(tree, struct tree);
cache.h +2 -1
@@ -1774,7 +1774,8 @@ extern void *alloc_commit_node_the_repository(void);extern void *alloc_tag_node_the_repository (void );#define alloc_object_node (r) alloc_object_node_##r()extern void *alloc_object_node_the_repository (void ); -extern void alloc_report (void ); +#define alloc_report (r) alloc_report_##r() +extern void alloc_report_the_repository (void );extern unsigned int alloc_commit_index (void );/* pkt-line.c */
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 10/13 alloc: add repository argument to alloc_commit_index
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 4 ++-- cache.h | 3 ++- object.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-)
alloc.c +2 -2
@@ -82,7 +82,7 @@ void *alloc_object_node_the_repository(void)static struct alloc_state commit_state; -unsigned int alloc_commit_index (void ) +unsigned int alloc_commit_index_the_repository (void ) {static unsigned int count;return count++;
@@ -92,7 +92,7 @@ void *alloc_commit_node_the_repository(void) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (); + c->index =alloc_commit_index (the_repository);return c; }
cache.h +2 -1
@@ -1776,7 +1776,8 @@ extern void *alloc_tag_node_the_repository(void);extern void *alloc_object_node_the_repository (void );#define alloc_report (r) alloc_report_##r()extern void alloc_report_the_repository (void ); -extern unsigned int alloc_commit_index (void ); +#define alloc_commit_index (r) alloc_commit_index_##r() +extern unsigned int alloc_commit_index_the_repository (void );/* pkt-line.c */ void packet_trace_identity (const char *prog);
object.c +1 -1
@@ -162,7 +162,7 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet)return obj; else if (obj->type == OBJ_NONE) {if (type ==OBJ_COMMIT ) - ((struct commit *)obj)->index =alloc_commit_index (); + ((struct commit *)obj)->index =alloc_commit_index (the_repository); obj->type = type;return obj; }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 11/13 object: allow grow_object_hash to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
object.c +8 -8
@@ -116,27 +116,27 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -#define grow_object_hash (r) grow_object_hash_##r() -static void grow_object_hash_the_repository (void ) +static void grow_object_hash (struct repository *r) {int i;/* * Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size; +int new_hash_size = r->parsed_objects->obj_hash_size <32 ?32 :2 * r->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { -struct object *obj = the_repository->parsed_objects->obj_hash[i]; +for (i =0 ; i < r->parsed_objects->obj_hash_size; i++) { +struct object *obj = r->parsed_objects->obj_hash[i]; +if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (the_repository->parsed_objects->obj_hash); - the_repository->parsed_objects->obj_hash = new_hash; - the_repository->parsed_objects->obj_hash_size = new_hash_size; +free (r->parsed_objects->obj_hash); + r->parsed_objects->obj_hash = new_hash; + r->parsed_objects->obj_hash_size = new_hash_size; }void *create_object_the_repository (const unsigned char *sha1,void *o)
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 12/13 object: allow create_object to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 12 ++++++------ object.h | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-)
object.c +6 -6
@@ -139,7 +139,7 @@ static void grow_object_hash(struct repository *r) r->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object_the_repository (const unsigned char *sha1,void *o) +void *create_object (struct repository *r,const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -147,12 +147,12 @@ void *create_object_the_repository(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (the_repository); +if (r->parsed_objects->obj_hash_size -1 <= r->parsed_objects->nr_objs *2 ) +grow_object_hash (r); -insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, - the_repository->parsed_objects->obj_hash_size); - the_repository->parsed_objects->nr_objs++; +insert_obj_hash (obj, r->parsed_objects->obj_hash, + r->parsed_objects->obj_hash_size); + r->parsed_objects->nr_objs++;return obj; }
object.h +1 -2
@@ -93,8 +93,7 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -#define create_object (r, s, o) create_object_##r(s, o) -extern void *create_object_the_repository (const unsigned char *sha1,void *obj); +extern void *create_object (struct repository *r,const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v3 13/13 alloc: allow arbitrary repositories for alloc functions
In reply to Stefan BellerWe have to convert all of the alloc functions at once, because alloc_report uses a funky macro for reporting. It is better for the sake of mechanical conversion to convert multiple functions at once rather than changing the structure of the reporting function.
We record all memory allocation in alloc.c, and free them in clear_alloc_state, which is called for all repositories except the_repository.
--- alloc.c | 79 +++++++++++++++++++++++++++++++++-------------- alloc.h | 23 ++++++++++++++ blame.c | 1 + blob.c | 1 + cache.h | 16 ---------- commit.c | 1 + merge-recursive.c | 1 + object.c | 37 ++++++++++++++++++++-- object.h | 8 +++++ tag.c | 1 + tree.c | 1 + 11 files changed, 127 insertions(+), 42 deletions(-) create mode 100644 alloc.h
alloc.c +55 -24
@@ -4,8 +4,7 @@* Copyright (C) 2006 Linus Torvalds * * The standard malloc/free wastes too much space for objects, partly because -* it maintains all the allocation infrastructure (which isn't needed, since -* we never free an object descriptor anyway), but even more because it ends +* it maintains all the allocation infrastructure, but even more because it ends * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. */
@@ -15,6 +14,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #define BLOCKING 1024
@@ -30,8 +30,25 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs; +int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + } +} +static inline void *alloc_node (struct alloc_state *s,size_t node_size) {void *ret;
@@ -39,63 +56,76 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)if (!s->nr) { s->nr =BLOCKING ; s->p =xmalloc (BLOCKING * node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = s->p; } s->nr--; s->count++; ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); +return ret; } -static struct alloc_state blob_state; -void *alloc_blob_node_the_repository (void ) +void *alloc_blob_node (struct repository *r) { -struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); +struct blob *b =alloc_node (r->parsed_objects->blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;return b; } -static struct alloc_state tree_state; -void *alloc_tree_node_the_repository (void ) +void *alloc_tree_node (struct repository *r) { -struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); +struct tree *t =alloc_node (r->parsed_objects->tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;return t; } -static struct alloc_state tag_state; -void *alloc_tag_node_the_repository (void ) +void *alloc_tag_node (struct repository *r) { -struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); +struct tag *t =alloc_node (r->parsed_objects->tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;return t; } -static struct alloc_state object_state; -void *alloc_object_node_the_repository (void ) +void *alloc_object_node (struct repository *r) { -struct object *obj =alloc_node (&object_state,sizeof (union any_object )); +struct object *obj =alloc_node (r->parsed_objects->object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;return obj; } -static struct alloc_state commit_state; - -unsigned int alloc_commit_index_the_repository (void ) +unsigned int alloc_commit_index (struct repository *r) { -static unsigned int count; -return count++; +return r->parsed_objects->commit_count++; } -void *alloc_commit_node_the_repository (void ) +void *alloc_commit_node (struct repository *r) { -struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); +struct commit *c =alloc_node (r->parsed_objects->commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (the_repository); + c->index =alloc_commit_index (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",
@@ -103,9 +133,10 @@ static void report(const char *name, unsigned int count, size_t size) }#define REPORT (name, type) \ - report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) + report(#name, r->parsed_objects->name##_state->count, \ + r->parsed_objects->name##_state->count * sizeof(type) >> 10) -void alloc_report_the_repository (void ) +void alloc_report (struct repository *r) { REPORT(blob, struct blob); REPORT(tree, struct tree);
alloc.h new file +23 -0
@@ -0,0 +1,23 @@ +#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); +void *alloc_tag_node (struct repository *r); +void *alloc_object_node (struct repository *r); +void alloc_report (struct repository *r); +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
blame.c +1 -0
@@ -6,6 +6,7 @@#include "diffcore.h" #include "tag.h" #include "blame.h" +#include "alloc.h" void blame_origin_decref (struct blame_origin *o) {
blob.c +1 -0
@@ -1,6 +1,7 @@#include "cache.h" #include "blob.h" #include "repository.h" +#include "alloc.h" const char *blob_type ="blob" ;
cache.h +0 -16
@@ -1763,22 +1763,6 @@ extern const char *excludes_file;int decode_85 (char *dst,const char *line,int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes); -/* alloc.c */ -#define alloc_blob_node (r) alloc_blob_node_##r() -extern void *alloc_blob_node_the_repository (void ); -#define alloc_tree_node (r) alloc_tree_node_##r() -extern void *alloc_tree_node_the_repository (void ); -#define alloc_commit_node (r) alloc_commit_node_##r() -extern void *alloc_commit_node_the_repository (void ); -#define alloc_tag_node (r) alloc_tag_node_##r() -extern void *alloc_tag_node_the_repository (void ); -#define alloc_object_node (r) alloc_object_node_##r() -extern void *alloc_object_node_the_repository (void ); -#define alloc_report (r) alloc_report_##r() -extern void alloc_report_the_repository (void ); -#define alloc_commit_index (r) alloc_commit_index_##r() -extern unsigned int alloc_commit_index_the_repository (void ); -/* pkt-line.c */ void packet_trace_identity (const char *prog);
commit.c +1 -0
@@ -6,6 +6,7 @@#include "diff.h" #include "revision.h" #include "notes.h" +#include "alloc.h" #include "gpg-interface.h" #include "mergesort.h" #include "commit-slab.h"
merge-recursive.c +1 -0
@@ -15,6 +15,7 @@#include "diff.h" #include "diffcore.h" #include "tag.h" +#include "alloc.h" #include "unpack-trees.h" #include "string-list.h" #include "xdiff-interface.h"
object.c +35 -2
@@ -5,6 +5,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "object-store.h" #include "packfile.h"
@@ -455,6 +456,13 @@ 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 (); +return o; }
@@ -501,9 +509,34 @@ void raw_object_store_clear(struct raw_object_store *o)void parsed_object_pool_clear (struct parsed_object_pool *o) {/* -* TOOD free objects in o->obj_hash. -* * As objects are allocated in slabs (see alloc.c), we do * not need to free each object, but each slab instead. +* +* Before doing so, we need to free any additional memory +* the objects may hold. */ +unsigned i; + +for (i =0 ; i < o->obj_hash_size; i++) { +struct object *obj = o->obj_hash[i]; + +if (!obj) +continue ; + +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); +clear_alloc_state (o->tag_state); +clear_alloc_state (o->object_state); }
object.h +8 -0
@@ -4,6 +4,14 @@struct parsed_object_pool {struct object **obj_hash;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; +unsigned commit_count; };struct parsed_object_pool *parsed_object_pool_new (void );
tag.c +1 -0
@@ -3,6 +3,7 @@#include "commit.h" #include "tree.h" #include "blob.h" +#include "alloc.h" #include "gpg-interface.h" const char *tag_type ="tag" ;
tree.c +1 -0
@@ -5,6 +5,7 @@#include "blob.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "tree-walk.h" const char *tree_type ="tree" ;
Signature · 1 line
2.17.0.255.g8bfb7c0704
Re: [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions
alloc.c struct alloc_state { · line 49
Quoted 6 lines · On Tue, 8 May 2018 12:37:36 -0700 Stefan Beller <[email protected]> wrote:
alloc.c +6
-0+void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + }
I should have caught this earlier, but you need to free s->slabs itself too.
alloc.h line 21
Quoted 3 lines
alloc.h new file +3
-0+void release_tree_node (struct tree *t); +void release_commit_node (struct commit *c); +void release_tag_node (struct tag *t);
Do these really need to be defined in alloc.c? I would think that it would be sufficient to define them as static in object.c.
Having said that, opinions differ (e.g. Duy said he thinks that release_ goes with alloc_ [1]) so I'm OK either way.
[1] https://public-inbox.org/git/CACsJy8D-e-bff3S+LQAMfwB-w8OpkjrfFrV9O5S3ku+M5aAjQA@mail.gmail.com/
Re: [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 12 lines · Jonathan Tan
alloc.c struct alloc_state { · line 49
Quoted 6 lines · On Tue, 8 May 2018 12:37:36 -0700 Stefan Beller <[email protected]> wrote:
alloc.c +6
-0+void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + }I should have caught this earlier, but you need to free
s->slabsitself too.
ok.
Quoted 10 lines
alloc.h line 21
Quoted 4 lines
alloc.h new file +3
-0+void release_tree_node (struct tree *t); +void release_commit_node (struct commit *c); +void release_tag_node (struct tag *t);Do these really need to be defined in
alloc.c? I would think that it would be sufficient to define them as static inobject.c.Having said that, opinions differ (e.g. Duy said he thinks that
release_goes withalloc_[1]) so I'm OK either way.
I would have preferred static as well, but went with Duys suggestion of having it in alloc.c.
I can change that.
Thanks,
Stefan
Re: [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 7 lines · Stefan Beller
Quoted 7 lines
alloc.h line 21
Quoted 3 lines
alloc.h new file +3
-0+void release_tree_node (struct tree *t); +void release_commit_node (struct commit *c); +void release_tag_node (struct tag *t);Do these really need to be defined in
alloc.c? I would think that it would be sufficient to define them as static inobject.c.
Quoted 7 lines
Quoted 2 lines
Having said that, opinions differ (e.g. Duy said he thinks that
release_goes withalloc_[1]) so I'm OK either way.I would have preferred static as well, but went with Duys suggestion of having it in
alloc.c.I can change that.
Heh I thought you would make them static ;-) I just wanted to keep release logic outside that object pool, which is clearer and also makes it easier to replace it with mem-pool.c later. I'm ok with making it static. Or if you do export these, please move them close to the parse_* functions where memory is actually allocated. E.g. release_commit_node() is moved to commit.c, close to parse_commit_gently(), release_tree_node() close to parse_tree_gently().
Signature · 1 line
Duy
Re: [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions
In reply to Jonathan TanQuoted 12 lines · Jonathan Tan
alloc.c struct alloc_state { · line 49
Quoted 6 lines · On Tue, 8 May 2018 12:37:36 -0700 Stefan Beller <[email protected]> wrote:
alloc.c +6
-0+void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + s->slab_nr--; +free (s->slabs[s->slab_nr]); + }I should have caught this earlier, but you need to free
s->slabsitself too.
And nobody frees 's' either. I'm not saying cler_alloc_state() should, but somebody should. When I tried repo_clear(the_repository) with gitster/sb/object-store-alloc I got this
==13250== 944 (32 direct, 912 indirect) bytes in 1 blocks are definitely lost in loss record 62 of 88
==13250== at 0x4C2CF25: calloc (vg_replace_malloc.c:718) ==13250== by 0x1AB7A8: xcalloc (wrapper.c:160) ==13250== by 0x1BF666: allocate_alloc_state (alloc.c:41) ==13250== by 0x140090: parsed_object_pool_new (object.c:462) ==13250== by 0x16CCF4: initialize_the_repository (repository.c:18) ==13250== by 0x110BD0: main (common-main.c:37)
If you want to reproduce, this is what I used to test this with.
https://gist.github.com/pclouds/86a2df6c28043f1b6fa3d4e72e7a1276
Signature · 1 line
Duy
Re: [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 4 lines · Duy Nguyen
If you want to reproduce, this is what I used to test this with.
https://gist.github.com/pclouds/86a2df6c28043f1b6fa3d4e72e7a1276
This only applied cleanly after I created an empty file at t/helper/test-abc.c, using git-apply. I'll use it to have no leaks here.
Thanks!
Stefan
Re: [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions
Quoted 8 lines · Stefan Beller
Quoted 4 lines · Duy Nguyen
If you want to reproduce, this is what I used to test this with.
https://gist.github.com/pclouds/86a2df6c28043f1b6fa3d4e72e7a1276
This only applied cleanly after I created an empty file at
t/helper/test-abc.c, using git-apply.
Right. I created the patch with "git add -N". I know exactly what the bug is but I'll need to be careful with renaming a bit before trying to fix this. Thanks for reminding me.
Signature · 1 line
Duy
v4 cover object store: alloc
In reply to Stefan Bellerv4:
* 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/[email protected]/
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
Signature · 1 line
2.17.0.255.g8bfb7c0704
alloc.c +2 -16
@@ -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",
alloc.h +0 -4
@@ -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
commit.c +7 -0
@@ -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);
commit.h +6 -0
@@ -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.
object.c +8 -3
@@ -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); }
tag.c +5 -0
@@ -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;
tag.h +1 -0
@@ -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,
PATCH v4 1/13 repository: introduce parsed objects field
Convert the existing global cache for parsed objects (obj_hash) into repository-specific parsed object caches. Existing code that uses obj_hash are modified to use the parsed object cache of the_repository; future patches will use the parsed object caches of other repositories.
Another future use case for a pool of objects is ease of memory management in revision walking: If we can free the rev-list related memory early in pack-objects (e.g. part of repack operation) then it could lower memory pressure significantly when running on large repos. While this has been discussed on the mailing list lately, this series doesn't implement this.
--- object.c | 63 +++++++++++++++++++++++++++++++++------------------- object.h | 8 +++++++ repository.c | 7 ++++++ repository.h | 9 ++++++++ 4 files changed, 64 insertions(+), 23 deletions(-)
object.c +40 -23
@@ -8,17 +8,14 @@#include "object-store.h" #include "packfile.h" -static struct object **obj_hash; -static int nr_objs, obj_hash_size; -unsigned int get_max_object_index (void ) { -return obj_hash_size; +return the_repository->parsed_objects->obj_hash_size; }struct object *get_indexed_object (unsigned int idx) { -return obj_hash[idx]; +return the_repository->parsed_objects->obj_hash[idx]; }static const char *object_type_strings[] = {
@@ -90,15 +87,16 @@ struct object *lookup_object(const unsigned char *sha1)unsigned int i, first;struct object *obj; -if (!obj_hash) +if (!the_repository->parsed_objects->obj_hash)return NULL ; - first = i =hash_obj (sha1, obj_hash_size); -while ((obj = obj_hash[i]) !=NULL ) { + first = i =hash_obj (sha1, + the_repository->parsed_objects->obj_hash_size); +while ((obj = the_repository->parsed_objects->obj_hash[i]) !=NULL ) {if (!hashcmp (sha1, obj->oid.hash))break ; i++; -if (i == obj_hash_size) +if (i == the_repository->parsed_objects->obj_hash_size) i =0 ; }if (obj && i != first) {
@@ -107,7 +105,8 @@ struct object *lookup_object(const unsigned char *sha1)* that we do not need to walk the hash table the next * time we look for it. */ -SWAP (obj_hash[i], obj_hash[first]); +SWAP (the_repository->parsed_objects->obj_hash[i], + the_repository->parsed_objects->obj_hash[first]); }return obj; }
@@ -124,19 +123,19 @@ static void grow_object_hash(void)* Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = obj_hash_size <32 ?32 :2 * obj_hash_size; +int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (obj_hash); - obj_hash = new_hash; - obj_hash_size = new_hash_size; +free (the_repository->parsed_objects->obj_hash); + the_repository->parsed_objects->obj_hash = new_hash; + the_repository->parsed_objects->obj_hash_size = new_hash_size; }void *create_object (const unsigned char *sha1,void *o)
@@ -147,11 +146,12 @@ void *create_object(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (obj_hash_size -1 <= nr_objs *2 ) +if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 )grow_object_hash (); -insert_obj_hash (obj, obj_hash, obj_hash_size); - nr_objs++; +insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, + the_repository->parsed_objects->obj_hash_size); + the_repository->parsed_objects->nr_objs++;return obj; }
@@ -431,8 +431,8 @@ void clear_object_flags(unsigned flags) {int i; -for (i=0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i=0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj) obj->flags &= ~flags; }
@@ -442,13 +442,20 @@ void clear_commit_marks_all(unsigned int flags) {int i; -for (i =0 ; i < obj_hash_size; i++) { -struct object *obj = obj_hash[i]; +for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { +struct object *obj = the_repository->parsed_objects->obj_hash[i];if (obj && obj->type ==OBJ_COMMIT ) obj->flags &= ~flags; } } +struct parsed_object_pool *parsed_object_pool_new (void ) +{ +struct parsed_object_pool *o =xmalloc (sizeof (*o)); +memset (o,0 ,sizeof (*o)); +return o; +} +struct raw_object_store *raw_object_store_new (void ) {struct raw_object_store *o =xmalloc (sizeof (*o));
@@ -488,3 +495,13 @@ void raw_object_store_clear(struct raw_object_store *o)close_all_packs (o); o->packed_git =NULL ; } + +void parsed_object_pool_clear (struct parsed_object_pool *o) +{ +/* +* TOOD free objects in o->obj_hash. +* +* As objects are allocated in slabs (see alloc.c), we do +* not need to free each object, but each slab instead. +*/ +}
object.h +8 -0
@@ -1,6 +1,14 @@ #ifndef OBJECT_H #define OBJECT_H +struct parsed_object_pool { + struct object **obj_hash; + int nr_objs, obj_hash_size; +}; + +struct parsed_object_pool *parsed_object_pool_new(void); +void parsed_object_pool_clear(struct parsed_object_pool *o); + struct object_list { struct object *item; struct object_list *next;
repository.c +7 -0
@@ -2,6 +2,7 @@#include "repository.h" #include "object-store.h" #include "config.h" +#include "object.h" #include "submodule-config.h" /* The main repository */
@@ -14,6 +15,8 @@ void initialize_the_repository(void) the_repo.index = &the_index; the_repo.objects =raw_object_store_new (); + the_repo.parsed_objects =parsed_object_pool_new (); +repo_set_hash_algo (&the_repo,GIT_HASH_SHA1 ); }
@@ -143,6 +146,7 @@ static int repo_init(struct repository *repo,memset (repo,0 ,sizeof (*repo)); repo->objects =raw_object_store_new (); + repo->parsed_objects =parsed_object_pool_new ();if (repo_init_gitdir (repo, gitdir)) goto error;
@@ -226,6 +230,9 @@ void repo_clear(struct repository *repo)raw_object_store_clear (repo->objects);FREE_AND_NULL (repo->objects); +parsed_object_pool_clear (repo->parsed_objects); +FREE_AND_NULL (repo->parsed_objects); +if (repo->config) {git_configset_clear (repo->config);FREE_AND_NULL (repo->config);
repository.h +9 -0
@@ -26,6 +26,15 @@ struct repository {*/ struct raw_object_store *objects; +/* +* All objects in this repository that have been parsed. This structure +* owns all objects it references, so users of "struct object *" +* generally do not need to free them; instead, when a repository is no +* longer used, call parsed_object_pool_clear() on this structure, which +* is called by the repositories repo_clear on its desconstruction. +*/ +struct parsed_object_pool *parsed_objects; +/* The store in which the refs are held. */ struct ref_store *refs;
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 2/13 object: add repository argument to create_object
In reply to Stefan BellerAdd a repository argument to allow the callers of create_object to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
--- blob.c | 4 +++- commit.c | 3 ++- object.c | 5 +++-- object.h | 3 ++- tag.c | 3 ++- tree.c | 3 ++- 6 files changed, 14 insertions(+), 7 deletions(-)
blob.c +3 -1
@@ -1,5 +1,6 @@#include "cache.h" #include "blob.h" +#include "repository.h" const char *blob_type ="blob" ;
@@ -7,7 +8,8 @@ struct blob *lookup_blob(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_blob_node ()); +return create_object (the_repository, oid->hash, +alloc_blob_node ());return object_as_type (obj,OBJ_BLOB ,0 ); }
commit.c +2 -1
@@ -50,7 +50,8 @@ struct commit *lookup_commit(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_commit_node ()); +return create_object (the_repository, oid->hash, +alloc_commit_node ());return object_as_type (obj,OBJ_COMMIT ,0 ); }
object.c +3 -2
@@ -138,7 +138,7 @@ static void grow_object_hash(void) the_repository->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object (const unsigned char *sha1,void *o) +void *create_object_the_repository (const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -178,7 +178,8 @@ struct object *lookup_unknown_object(const unsigned char *sha1) {struct object *obj =lookup_object (sha1);if (!obj) - obj =create_object (sha1,alloc_object_node ()); + obj =create_object (the_repository, sha1, +alloc_object_node ());return obj; }
object.h +2 -1
@@ -93,7 +93,8 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -extern void *create_object (const unsigned char *sha1,void *obj); +#define create_object (r, s, o) create_object_##r(s, o) +extern void *create_object_the_repository (const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
tag.c +2 -1
@@ -93,7 +93,8 @@ struct tag *lookup_tag(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tag_node ()); +return create_object (the_repository, oid->hash, +alloc_tag_node ());return object_as_type (obj,OBJ_TAG ,0 ); }
tree.c +2 -1
@@ -196,7 +196,8 @@ struct tree *lookup_tree(const struct object_id *oid) {struct object *obj =lookup_object (oid->hash);if (!obj) -return create_object (oid->hash,alloc_tree_node ()); +return create_object (the_repository, oid->hash, +alloc_tree_node ());return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 3/13 object: add repository argument to grow_object_hash
In reply to Stefan BellerFrom: Jonathan Nieder <[email protected]>
Add a repository argument to allow the caller of grow_object_hash to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
--- object.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
object.c +3 -2
@@ -116,7 +116,8 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -static void grow_object_hash (void ) +#define grow_object_hash (r) grow_object_hash_##r() +static void grow_object_hash_the_repository (void ) {int i;/*
@@ -147,7 +148,7 @@ void *create_object_the_repository(const unsigned char *sha1, void *o)hashcpy (obj->oid.hash, sha1);if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (); +grow_object_hash (the_repository);insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, the_repository->parsed_objects->obj_hash_size);
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 4/13 alloc: add repository argument to alloc_blob_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- blob.c | 2 +- cache.h | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -49,7 +49,7 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size) }static struct alloc_state blob_state; -void *alloc_blob_node (void ) +void *alloc_blob_node_the_repository (void ) {struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;
blob.c +1 -1
@@ -9,7 +9,7 @@ struct blob *lookup_blob(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_blob_node ()); +alloc_blob_node (the_repository));return object_as_type (obj,OBJ_BLOB ,0 ); }
cache.h +2 -1
@@ -1764,7 +1764,8 @@ int decode_85(char *dst, const char *line, int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes);/* alloc.c */ -extern void *alloc_blob_node (void ); +#define alloc_blob_node (r) alloc_blob_node_##r() +extern void *alloc_blob_node_the_repository (void );extern void *alloc_tree_node (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 5/13 alloc: add repository argument to alloc_tree_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- tree.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -57,7 +57,7 @@ void *alloc_blob_node_the_repository(void) }static struct alloc_state tree_state; -void *alloc_tree_node (void ) +void *alloc_tree_node_the_repository (void ) {struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;
cache.h +2 -1
@@ -1766,7 +1766,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);/* alloc.c */ #define alloc_blob_node (r) alloc_blob_node_##r()extern void *alloc_blob_node_the_repository (void ); -extern void *alloc_tree_node (void ); +#define alloc_tree_node (r) alloc_tree_node_##r() +extern void *alloc_tree_node_the_repository (void );extern void *alloc_commit_node (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );
tree.c +1 -1
@@ -197,7 +197,7 @@ struct tree *lookup_tree(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tree_node ()); +alloc_tree_node (the_repository));return object_as_type (obj,OBJ_TREE ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 6/13 alloc: add repository argument to alloc_commit_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- blame.c | 2 +- cache.h | 3 ++- commit.c | 2 +- merge-recursive.c | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-)
alloc.c +1 -1
@@ -88,7 +88,7 @@ unsigned int alloc_commit_index(void)return count++; } -void *alloc_commit_node (void ) +void *alloc_commit_node_the_repository (void ) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ;
blame.c +1 -1
@@ -161,7 +161,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,read_cache ();time (&now); - commit =alloc_commit_node (); + commit =alloc_commit_node (the_repository); commit->object.parsed =1 ; commit->date = now; parent_tail = &commit->parents;
cache.h +2 -1
@@ -1768,7 +1768,8 @@ void encode_85(char *buf, const unsigned char *data, int bytes);extern void *alloc_blob_node_the_repository (void );#define alloc_tree_node (r) alloc_tree_node_##r()extern void *alloc_tree_node_the_repository (void ); -extern void *alloc_commit_node (void ); +#define alloc_commit_node (r) alloc_commit_node_##r() +extern void *alloc_commit_node_the_repository (void );extern void *alloc_tag_node (void );extern void *alloc_object_node (void );extern void alloc_report (void );
commit.c +1 -1
@@ -51,7 +51,7 @@ struct commit *lookup_commit(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_commit_node ()); +alloc_commit_node (the_repository));return object_as_type (obj,OBJ_COMMIT ,0 ); }
merge-recursive.c +1 -1
@@ -98,7 +98,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two,static struct commit *make_virtual_commit (struct tree *tree,const char *comment) { -struct commit *commit =alloc_commit_node (); +struct commit *commit =alloc_commit_node (the_repository);set_merge_remote_desc (commit, comment, (struct object *)commit); commit->tree = tree;
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 7/13 alloc: add repository argument to alloc_tag_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- tag.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -65,7 +65,7 @@ void *alloc_tree_node_the_repository(void) }static struct alloc_state tag_state; -void *alloc_tag_node (void ) +void *alloc_tag_node_the_repository (void ) {struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;
cache.h +2 -1
@@ -1770,7 +1770,8 @@ extern void *alloc_blob_node_the_repository(void);extern void *alloc_tree_node_the_repository (void );#define alloc_commit_node (r) alloc_commit_node_##r()extern void *alloc_commit_node_the_repository (void ); -extern void *alloc_tag_node (void ); +#define alloc_tag_node (r) alloc_tag_node_##r() +extern void *alloc_tag_node_the_repository (void );extern void *alloc_object_node (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
tag.c +1 -1
@@ -94,7 +94,7 @@ struct tag *lookup_tag(const struct object_id *oid)struct object *obj =lookup_object (oid->hash);if (!obj)return create_object (the_repository, oid->hash, -alloc_tag_node ()); +alloc_tag_node (the_repository));return object_as_type (obj,OBJ_TAG ,0 ); }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 8/13 alloc: add repository argument to alloc_object_node
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- object.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
alloc.c +1 -1
@@ -73,7 +73,7 @@ void *alloc_tag_node_the_repository(void) }static struct alloc_state object_state; -void *alloc_object_node (void ) +void *alloc_object_node_the_repository (void ) {struct object *obj =alloc_node (&object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;
cache.h +2 -1
@@ -1772,7 +1772,8 @@ extern void *alloc_tree_node_the_repository(void);extern void *alloc_commit_node_the_repository (void );#define alloc_tag_node (r) alloc_tag_node_##r()extern void *alloc_tag_node_the_repository (void ); -extern void *alloc_object_node (void ); +#define alloc_object_node (r) alloc_object_node_##r() +extern void *alloc_object_node_the_repository (void );extern void alloc_report (void );extern unsigned int alloc_commit_index (void );
object.c +1 -1
@@ -180,7 +180,7 @@ struct object *lookup_unknown_object(const unsigned char *sha1)struct object *obj =lookup_object (sha1);if (!obj) obj =create_object (the_repository, sha1, -alloc_object_node ()); +alloc_object_node (the_repository));return obj; }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 9/13 alloc: add repository argument to alloc_report
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 2 +- cache.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
alloc.c +1 -1
@@ -105,7 +105,7 @@ static void report(const char *name, unsigned int count, size_t size)#define REPORT (name, type) \ report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) -void alloc_report (void ) +void alloc_report_the_repository (void ) { REPORT(blob, struct blob); REPORT(tree, struct tree);
cache.h +2 -1
@@ -1774,7 +1774,8 @@ extern void *alloc_commit_node_the_repository(void);extern void *alloc_tag_node_the_repository (void );#define alloc_object_node (r) alloc_object_node_##r()extern void *alloc_object_node_the_repository (void ); -extern void alloc_report (void ); +#define alloc_report (r) alloc_report_##r() +extern void alloc_report_the_repository (void );extern unsigned int alloc_commit_index (void );/* pkt-line.c */
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 10/13 alloc: add repository argument to alloc_commit_index
In reply to Stefan BellerThis is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. Use a macro to catch callers passing a repository other than the_repository at compile time.
--- alloc.c | 4 ++-- cache.h | 3 ++- object.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-)
alloc.c +2 -2
@@ -82,7 +82,7 @@ void *alloc_object_node_the_repository(void)static struct alloc_state commit_state; -unsigned int alloc_commit_index (void ) +unsigned int alloc_commit_index_the_repository (void ) {static unsigned int count;return count++;
@@ -92,7 +92,7 @@ void *alloc_commit_node_the_repository(void) {struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (); + c->index =alloc_commit_index (the_repository);return c; }
cache.h +2 -1
@@ -1776,7 +1776,8 @@ extern void *alloc_tag_node_the_repository(void);extern void *alloc_object_node_the_repository (void );#define alloc_report (r) alloc_report_##r()extern void alloc_report_the_repository (void ); -extern unsigned int alloc_commit_index (void ); +#define alloc_commit_index (r) alloc_commit_index_##r() +extern unsigned int alloc_commit_index_the_repository (void );/* pkt-line.c */ void packet_trace_identity (const char *prog);
object.c +1 -1
@@ -162,7 +162,7 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet)return obj; else if (obj->type == OBJ_NONE) {if (type ==OBJ_COMMIT ) - ((struct commit *)obj)->index =alloc_commit_index (); + ((struct commit *)obj)->index =alloc_commit_index (the_repository); obj->type = type;return obj; }
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 11/13 object: allow grow_object_hash to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
object.c +8 -8
@@ -116,27 +116,27 @@ struct object *lookup_object(const unsigned char *sha1)* power of 2 (but at least 32). Copy the existing values to the new * hash map. */ -#define grow_object_hash (r) grow_object_hash_##r() -static void grow_object_hash_the_repository (void ) +static void grow_object_hash (struct repository *r) {int i;/* * Note that this size must always be power-of-2 to match hash_obj * above. */ -int new_hash_size = the_repository->parsed_objects->obj_hash_size <32 ?32 :2 * the_repository->parsed_objects->obj_hash_size; +int new_hash_size = r->parsed_objects->obj_hash_size <32 ?32 :2 * r->parsed_objects->obj_hash_size;struct object **new_hash; new_hash =xcalloc (new_hash_size,sizeof (struct object *)); -for (i =0 ; i < the_repository->parsed_objects->obj_hash_size; i++) { -struct object *obj = the_repository->parsed_objects->obj_hash[i]; +for (i =0 ; i < r->parsed_objects->obj_hash_size; i++) { +struct object *obj = r->parsed_objects->obj_hash[i]; +if (!obj)continue ;insert_obj_hash (obj, new_hash, new_hash_size); } -free (the_repository->parsed_objects->obj_hash); - the_repository->parsed_objects->obj_hash = new_hash; - the_repository->parsed_objects->obj_hash_size = new_hash_size; +free (r->parsed_objects->obj_hash); + r->parsed_objects->obj_hash = new_hash; + r->parsed_objects->obj_hash_size = new_hash_size; }void *create_object_the_repository (const unsigned char *sha1,void *o)
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 12/13 object: allow create_object to handle arbitrary repositories
In reply to Stefan Beller--- object.c | 12 ++++++------ object.h | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-)
object.c +6 -6
@@ -139,7 +139,7 @@ static void grow_object_hash(struct repository *r) r->parsed_objects->obj_hash_size = new_hash_size; } -void *create_object_the_repository (const unsigned char *sha1,void *o) +void *create_object (struct repository *r,const unsigned char *sha1,void *o) {struct object *obj = o;
@@ -147,12 +147,12 @@ void *create_object_the_repository(const unsigned char *sha1, void *o) obj->flags =0 ;hashcpy (obj->oid.hash, sha1); -if (the_repository->parsed_objects->obj_hash_size -1 <= the_repository->parsed_objects->nr_objs *2 ) -grow_object_hash (the_repository); +if (r->parsed_objects->obj_hash_size -1 <= r->parsed_objects->nr_objs *2 ) +grow_object_hash (r); -insert_obj_hash (obj, the_repository->parsed_objects->obj_hash, - the_repository->parsed_objects->obj_hash_size); - the_repository->parsed_objects->nr_objs++; +insert_obj_hash (obj, r->parsed_objects->obj_hash, + r->parsed_objects->obj_hash_size); + r->parsed_objects->nr_objs++;return obj; }
object.h +1 -2
@@ -93,8 +93,7 @@ extern struct object *get_indexed_object(unsigned int);*/ struct object *lookup_object (const unsigned char *sha1); -#define create_object (r, s, o) create_object_##r(s, o) -extern void *create_object_the_repository (const unsigned char *sha1,void *obj); +extern void *create_object (struct repository *r,const unsigned char *sha1,void *obj);void *object_as_type (struct object *obj,enum object_type type,int quiet);
Signature · 1 line
2.17.0.255.g8bfb7c0704
PATCH v4 13/13 alloc: allow arbitrary repositories for alloc functions
In reply to Stefan BellerWe have to convert all of the alloc functions at once, because alloc_report uses a funky macro for reporting. It is better for the sake of mechanical conversion to convert multiple functions at once rather than changing the structure of the reporting function.
We record all memory allocation in alloc.c, and free them in clear_alloc_state, which is called for all repositories except the_repository.
--- alloc.c | 65 ++++++++++++++++++++++++++++++----------------- alloc.h | 19 ++++++++++++++ blame.c | 1 + blob.c | 1 + cache.h | 16 ------------ commit.c | 8 ++++++ commit.h | 6 +++++ merge-recursive.c | 1 + object.c | 42 ++++++++++++++++++++++++++++-- object.h | 8 ++++++ tag.c | 6 +++++ tag.h | 1 + tree.c | 1 + 13 files changed, 133 insertions(+), 42 deletions(-) create mode 100644 alloc.h
alloc.c +41 -24
@@ -4,8 +4,7 @@* Copyright (C) 2006 Linus Torvalds * * The standard malloc/free wastes too much space for objects, partly because -* it maintains all the allocation infrastructure (which isn't needed, since -* we never free an object descriptor anyway), but even more because it ends +* it maintains all the allocation infrastructure, but even more because it ends * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. */
@@ -15,6 +14,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #define BLOCKING 1024
@@ -30,8 +30,27 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs; +int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + 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) {void *ret;
@@ -39,60 +58,57 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)if (!s->nr) { s->nr =BLOCKING ; s->p =xmalloc (BLOCKING * node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = s->p; } s->nr--; s->count++; ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); +return ret; } -static struct alloc_state blob_state; -void *alloc_blob_node_the_repository (void ) +void *alloc_blob_node (struct repository *r) { -struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); +struct blob *b =alloc_node (r->parsed_objects->blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;return b; } -static struct alloc_state tree_state; -void *alloc_tree_node_the_repository (void ) +void *alloc_tree_node (struct repository *r) { -struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); +struct tree *t =alloc_node (r->parsed_objects->tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;return t; } -static struct alloc_state tag_state; -void *alloc_tag_node_the_repository (void ) +void *alloc_tag_node (struct repository *r) { -struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); +struct tag *t =alloc_node (r->parsed_objects->tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;return t; } -static struct alloc_state object_state; -void *alloc_object_node_the_repository (void ) +void *alloc_object_node (struct repository *r) { -struct object *obj =alloc_node (&object_state,sizeof (union any_object )); +struct object *obj =alloc_node (r->parsed_objects->object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;return obj; } -static struct alloc_state commit_state; - -unsigned int alloc_commit_index_the_repository (void ) +unsigned int alloc_commit_index (struct repository *r) { -static unsigned int count; -return count++; +return r->parsed_objects->commit_count++; } -void *alloc_commit_node_the_repository (void ) +void *alloc_commit_node (struct repository *r) { -struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); +struct commit *c =alloc_node (r->parsed_objects->commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (the_repository); + c->index =alloc_commit_index (r);return c; }
@@ -103,9 +119,10 @@ static void report(const char *name, unsigned int count, size_t size) }#define REPORT (name, type) \ - report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) + report(#name, r->parsed_objects->name##_state->count, \ + r->parsed_objects->name##_state->count * sizeof(type) >> 10) -void alloc_report_the_repository (void ) +void alloc_report (struct repository *r) { REPORT(blob, struct blob); REPORT(tree, struct tree);
alloc.h new file +19 -0
@@ -0,0 +1,19 @@ +#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); +void *alloc_tag_node (struct repository *r); +void *alloc_object_node (struct repository *r); +void alloc_report (struct repository *r); +unsigned int alloc_commit_index (struct repository *r); + +void *allocate_alloc_state (void ); +void clear_alloc_state (struct alloc_state *s); + +#endif
blame.c +1 -0
@@ -6,6 +6,7 @@#include "diffcore.h" #include "tag.h" #include "blame.h" +#include "alloc.h" void blame_origin_decref (struct blame_origin *o) {
blob.c +1 -0
@@ -1,6 +1,7 @@#include "cache.h" #include "blob.h" #include "repository.h" +#include "alloc.h" const char *blob_type ="blob" ;
cache.h +0 -16
@@ -1763,22 +1763,6 @@ extern const char *excludes_file;int decode_85 (char *dst,const char *line,int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes); -/* alloc.c */ -#define alloc_blob_node (r) alloc_blob_node_##r() -extern void *alloc_blob_node_the_repository (void ); -#define alloc_tree_node (r) alloc_tree_node_##r() -extern void *alloc_tree_node_the_repository (void ); -#define alloc_commit_node (r) alloc_commit_node_##r() -extern void *alloc_commit_node_the_repository (void ); -#define alloc_tag_node (r) alloc_tag_node_##r() -extern void *alloc_tag_node_the_repository (void ); -#define alloc_object_node (r) alloc_object_node_##r() -extern void *alloc_object_node_the_repository (void ); -#define alloc_report (r) alloc_report_##r() -extern void alloc_report_the_repository (void ); -#define alloc_commit_index (r) alloc_commit_index_##r() -extern unsigned int alloc_commit_index_the_repository (void ); -/* pkt-line.c */ void packet_trace_identity (const char *prog);
commit.c +8 -0
@@ -6,6 +6,7 @@#include "diff.h" #include "revision.h" #include "notes.h" +#include "alloc.h" #include "gpg-interface.h" #include "mergesort.h" #include "commit-slab.h"
@@ -296,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);
commit.h +6 -0
@@ -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.
merge-recursive.c +1 -0
@@ -15,6 +15,7 @@#include "diff.h" #include "diffcore.h" #include "tag.h" +#include "alloc.h" #include "unpack-trees.h" #include "string-list.h" #include "xdiff-interface.h"
object.c +40 -2
61 lines not on this page · raw message
object.h +8 -0
@@ -4,6 +4,14 @@struct parsed_object_pool {struct object **obj_hash;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; +unsigned commit_count; };struct parsed_object_pool *parsed_object_pool_new (void );
tag.c +6 -0
@@ -3,6 +3,7 @@#include "commit.h" #include "tree.h" #include "blob.h" +#include "alloc.h" #include "gpg-interface.h" const char *tag_type ="tag" ;
@@ -115,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;
tag.h +1 -0
@@ -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,
tree.c +1 -0
@@ -5,6 +5,7 @@#include "blob.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "tree-walk.h" const char *tree_type ="tree" ;
Signature · 1 line
2.17.0.255.g8bfb7c0704
object.c void parsed_object_pool_clear(struct parsed_object_pool *o) · line 531
Quoted 9 lines · On Wed, 9 May 2018 17:40:11 -0700 Stefan Beller <[email protected]> wrote:
object.c +3
-3if (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);
This might seem a bit bikesheddy, but I wouldn't call it free_tag_buffer(), since what's being freed is not the buffer of the object itself, but just a string. If you want such a function, I would just call it release_tag_memory() to match release_commit_memory().
Other than that, all the patches look fine to me.
Some optional comments (this is almost certainly bikeshedding):
- I would call them release_commit() and release_tag(), to match strbuf_release().
- It might be better to just inline the handling of releasing commit and tag memory. This code already knows that, for a tree, it needs to free its buffer and only its buffer, so it is not much of a stretch to think that it similarly knows the details of commit and tag objects too.
Quoted 13 lines · Jonathan Tan
object.c void parsed_object_pool_clear(struct parsed_object_pool *o) · line 531
Quoted 9 lines · On Wed, 9 May 2018 17:40:11 -0700 Stefan Beller <[email protected]> wrote:
if (obj->type == OBJ_TREE)object.c +3
-3-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);
Quoted 8 lines
This might seem a bit bikesheddy, but I wouldn't call it
free_tag_buffer(), since what's being freed is not the buffer of the object itself, but just a string. If you want such a function, I would just call itrelease_tag_memory()to matchrelease_commit_memory().Other than that, all the patches look fine to me.
Some optional comments (this is almost certainly bikeshedding):
Who doesn't love some bikeshedding in late spring?
Quoted 3 lines
- I would call them
release_commit()andrelease_tag(), to matchstrbuf_release().
Why not commit_release and tag_release to also have the same order of words as in strbuf_release ?
Quoted 5 lines
- It might be better to just inline the handling of releasing commit and tag memory. This code already knows that, for a tree, it needs to free its buffer and only its buffer, so it is not much of a stretch to think that it similarly knows the details of commit and tag objects too.
We still call out to free_tree_buffer? Not sure I understand.
Quoted 5 lines · On Thu, 10 May 2018 10:32:09 -0700 Stefan Beller <[email protected]> wrote:
Quoted 2 lines
- I would call them
release_commit()andrelease_tag(), to matchstrbuf_release().Why not
commit_releaseandtag_releaseto also have the same order of words as instrbuf_release?
At this point in the discussion, either is fine.
Quoted 7 lines
Quoted 5 lines
- It might be better to just inline the handling of releasing commit and tag memory. This code already knows that, for a tree, it needs to free its buffer and only its buffer, so it is not much of a stretch to think that it similarly knows the details of commit and tag objects too.
We still call out to
free_tree_buffer? Not sure I understand.
I meant that since we call out to free_tree_buffer (as you said), this shows that the code knows the internal details of a tree object (in that it has a buffer, and that needs to be freed, and that is the only thing that needs to be freed), so maybe the code should operate on the internal details of commits and tags as well. But again, this is a minor point.
Quoted 10 lines · Jonathan Tan
Quoted 5 lines · On Thu, 10 May 2018 10:32:09 -0700 Stefan Beller <[email protected]> wrote:
Quoted 2 lines
- I would call them
release_commit()andrelease_tag(), to matchstrbuf_release().Why not
commit_releaseandtag_releaseto also have the same order of words as instrbuf_release?At this point in the discussion, either is fine.
ok, please express your opinion in form of a patch. ;)
I do not plan on resending this series unless someone comments something that they themselves do not consider bikesheddy.
Stefan
PATCH 1/1 alloc: allow arbitrary repositories for alloc functions
In reply to Jonathan TanWe have to convert all of the alloc functions at once, because alloc_report uses a funky macro for reporting. It is better for the sake of mechanical conversion to convert multiple functions at once rather than changing the structure of the reporting function.
We record all memory allocation in alloc.c, and free them in clear_alloc_state, which is called for all repositories except the_repository.
---
Quoted 4 lines
This might seem a bit bikesheddy, but I wouldn't call it
free_tag_buffer(), since what's being freed is not the buffer of the object itself, but just a string. If you want such a function, I would just call itrelease_tag_memory()to matchrelease_commit_memory().
So you would replace the last commit with a patch like this?
Thanks,
Stefan
Notes:
diff to what is currently queued:
diff --git c/commit.c w/commit.c
index 612ccf7b053..f3a5872c393 100644
--- c/commit.c
+++ w/commit.c
@@ -297,11 +297,15 @@ void free_commit_buffer(struct commit *commit)
}
}
-void release_commit_memory(struct commit *c)
+void relase_commit_memory(struct commit *c)
{
+ c->tree = NULL;
+ c->index = 0;
free_commit_buffer(c);
free_commit_list(c->parents);
/* TODO: what about commit->util? */
+
+ c->object.parsed = 0;
}
const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
diff --git c/commit.h w/commit.h
index 2d764ab7d8e..366c151e0cb 100644
--- c/commit.h
+++ w/commit.h
@@ -103,7 +103,7 @@ 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);
+void relase_commit_memory(struct commit *c);
/*
* Disassociate any cached object buffer from the commit, but do not free it.
diff --git c/object.c w/object.c
index 9d5b10d5a20..a7d1fd4a20b 100644
--- c/object.c
+++ w/object.c
@@ -526,9 +526,9 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
if (obj->type == OBJ_TREE)
free_tree_buffer((struct tree*)obj);
else if (obj->type == OBJ_COMMIT)
- release_commit_memory((struct commit*)obj);
+ relase_commit_memory((struct commit*)obj);
else if (obj->type == OBJ_TAG)
- free_tag_buffer((struct tag*)obj);
+ release_tag_memory((struct tag*)obj);
}
FREE_AND_NULL(o->obj_hash);
diff --git c/tag.c w/tag.c
index 254352c30c6..7c12426b4ea 100644
--- c/tag.c
+++ w/tag.c
@@ -116,9 +116,12 @@ 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)
+void release_tag_memory(struct tag *t)
{
free(t->tag);
+ t->tagged = NULL;
+ t->object.parsed = 0;
+ t->date = 0;
}
int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)
diff --git c/tag.h w/tag.h
index b241fe67bc5..9057d76a506 100644
--- c/tag.h
+++ w/tag.h
@@ -15,7 +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 void release_tag_memory(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,alloc.c | 65 ++++++++++++++++++++++++++++++----------------- alloc.h | 19 ++++++++++++++ blame.c | 1 + blob.c | 1 + cache.h | 16 ------------ commit.c | 12 +++++++++ commit.h | 6 +++++ merge-recursive.c | 1 + object.c | 42 ++++++++++++++++++++++++++++-- object.h | 8 ++++++ tag.c | 9 +++++++ tag.h | 1 + tree.c | 1 + 13 files changed, 140 insertions(+), 42 deletions(-) create mode 100644 alloc.h
alloc.c +41 -24
@@ -4,8 +4,7 @@* Copyright (C) 2006 Linus Torvalds * * The standard malloc/free wastes too much space for objects, partly because -* it maintains all the allocation infrastructure (which isn't needed, since -* we never free an object descriptor anyway), but even more because it ends +* it maintains all the allocation infrastructure, but even more because it ends * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. */
@@ -15,6 +14,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #define BLOCKING 1024
@@ -30,8 +30,27 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs; +int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + 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) {void *ret;
@@ -39,60 +58,57 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)if (!s->nr) { s->nr =BLOCKING ; s->p =xmalloc (BLOCKING * node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = s->p; } s->nr--; s->count++; ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); +return ret; } -static struct alloc_state blob_state; -void *alloc_blob_node_the_repository (void ) +void *alloc_blob_node (struct repository *r) { -struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); +struct blob *b =alloc_node (r->parsed_objects->blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;return b; } -static struct alloc_state tree_state; -void *alloc_tree_node_the_repository (void ) +void *alloc_tree_node (struct repository *r) { -struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); +struct tree *t =alloc_node (r->parsed_objects->tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;return t; } -static struct alloc_state tag_state; -void *alloc_tag_node_the_repository (void ) +void *alloc_tag_node (struct repository *r) { -struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); +struct tag *t =alloc_node (r->parsed_objects->tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;return t; } -static struct alloc_state object_state; -void *alloc_object_node_the_repository (void ) +void *alloc_object_node (struct repository *r) { -struct object *obj =alloc_node (&object_state,sizeof (union any_object )); +struct object *obj =alloc_node (r->parsed_objects->object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;return obj; } -static struct alloc_state commit_state; - -unsigned int alloc_commit_index_the_repository (void ) +unsigned int alloc_commit_index (struct repository *r) { -static unsigned int count; -return count++; +return r->parsed_objects->commit_count++; } -void *alloc_commit_node_the_repository (void ) +void *alloc_commit_node (struct repository *r) { -struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); +struct commit *c =alloc_node (r->parsed_objects->commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (the_repository); + c->index =alloc_commit_index (r);return c; }
@@ -103,9 +119,10 @@ static void report(const char *name, unsigned int count, size_t size) }#define REPORT (name, type) \ - report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) + report(#name, r->parsed_objects->name##_state->count, \ + r->parsed_objects->name##_state->count * sizeof(type) >> 10) -void alloc_report_the_repository (void ) +void alloc_report (struct repository *r) { REPORT(blob, struct blob); REPORT(tree, struct tree);
alloc.h new file +19 -0
@@ -0,0 +1,19 @@ +#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); +void *alloc_tag_node (struct repository *r); +void *alloc_object_node (struct repository *r); +void alloc_report (struct repository *r); +unsigned int alloc_commit_index (struct repository *r); + +void *allocate_alloc_state (void ); +void clear_alloc_state (struct alloc_state *s); + +#endif
blame.c +1 -0
@@ -6,6 +6,7 @@#include "diffcore.h" #include "tag.h" #include "blame.h" +#include "alloc.h" void blame_origin_decref (struct blame_origin *o) {
blob.c +1 -0
@@ -1,6 +1,7 @@#include "cache.h" #include "blob.h" #include "repository.h" +#include "alloc.h" const char *blob_type ="blob" ;
cache.h +0 -16
@@ -1763,22 +1763,6 @@ extern const char *excludes_file;int decode_85 (char *dst,const char *line,int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes); -/* alloc.c */ -#define alloc_blob_node (r) alloc_blob_node_##r() -extern void *alloc_blob_node_the_repository (void ); -#define alloc_tree_node (r) alloc_tree_node_##r() -extern void *alloc_tree_node_the_repository (void ); -#define alloc_commit_node (r) alloc_commit_node_##r() -extern void *alloc_commit_node_the_repository (void ); -#define alloc_tag_node (r) alloc_tag_node_##r() -extern void *alloc_tag_node_the_repository (void ); -#define alloc_object_node (r) alloc_object_node_##r() -extern void *alloc_object_node_the_repository (void ); -#define alloc_report (r) alloc_report_##r() -extern void alloc_report_the_repository (void ); -#define alloc_commit_index (r) alloc_commit_index_##r() -extern unsigned int alloc_commit_index_the_repository (void ); -/* pkt-line.c */ void packet_trace_identity (const char *prog);
commit.c +12 -0
@@ -6,6 +6,7 @@#include "diff.h" #include "revision.h" #include "notes.h" +#include "alloc.h" #include "gpg-interface.h" #include "mergesort.h" #include "commit-slab.h"
@@ -296,6 +297,17 @@ void free_commit_buffer(struct commit *commit) } } +void relase_commit_memory (struct commit *c) +{ + c->tree =NULL ; + c->index =0 ; +free_commit_buffer (c); +free_commit_list (c->parents); +/* TODO: what about commit->util? */ + + c->object.parsed =0 ; +} +const void *detach_commit_buffer (struct commit *commit,unsigned long *sizep) {struct commit_buffer *v =buffer_slab_peek (&buffer_slab, commit);
commit.h +6 -0
@@ -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 relase_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.
merge-recursive.c +1 -0
@@ -15,6 +15,7 @@#include "diff.h" #include "diffcore.h" #include "tag.h" +#include "alloc.h" #include "unpack-trees.h" #include "string-list.h" #include "xdiff-interface.h"
object.c +40 -2
61 lines not on this page · raw message
object.h +8 -0
@@ -4,6 +4,14 @@struct parsed_object_pool {struct object **obj_hash;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; +unsigned commit_count; };struct parsed_object_pool *parsed_object_pool_new (void );
tag.c +9 -0
@@ -3,6 +3,7 @@#include "commit.h" #include "tree.h" #include "blob.h" +#include "alloc.h" #include "gpg-interface.h" const char *tag_type ="tag" ;
@@ -115,6 +116,14 @@ static timestamp_t parse_tag_date(const char *buf, const char *tail)return parse_timestamp (dateptr,NULL ,10 ); } +void release_tag_memory (struct tag *t) +{ +free (t->tag); + t->tagged =NULL ; + t->object.parsed =0 ; + t->date =0 ; +} +int parse_tag_buffer (struct tag *item,const void *data,unsigned long size) {struct object_id oid;
tag.h +1 -0
@@ -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 release_tag_memory (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,
tree.c +1 -0
@@ -5,6 +5,7 @@#include "blob.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "tree-walk.h" const char *tree_type ="tree" ;
Signature · 1 line
2.17.0.255.g8bfb7c0704
Re: [PATCH] alloc: allow arbitrary repositories for alloc functions
commit.c void free_commit_buffer(struct commit *commit) · line 297
Quoted 3 lines · Stefan Beller
commit.c +1
-0@@ -296,6 +297,17 @@ void free_commit_buffer(struct commit *commit) +void relase_commit_memory (struct commit *c)
s/relase/release/
PATCH 1/1 alloc: allow arbitrary repositories for alloc functions
We have to convert all of the alloc functions at once, because alloc_report uses a funky macro for reporting. It is better for the sake of mechanical conversion to convert multiple functions at once rather than changing the structure of the reporting function.
We record all memory allocation in alloc.c, and free them in clear_alloc_state, which is called for all repositories except the_repository.
---
Notes:
Eric, I have fixed s/relase/release/
Jonathan,
Quoted 4 lines
This might seem a bit bikesheddy, but I wouldn't call it
free_tag_buffer(), since what's being freed is not the buffer of the object itself, but just a string. If you want such a function, I would just call itrelease_tag_memory()to matchrelease_commit_memory().
So you would replace the last commit with a patch like this?
Thanks,
Stefan
diff to what is currently queued:
diff --git c/commit.c w/commit.c
index 612ccf7b053..5eb4d2f08f8 100644
--- c/commit.c
+++ w/commit.c
@@ -299,9 +299,13 @@ void free_commit_buffer(struct commit *commit)
void release_commit_memory(struct commit *c)
{
+ c->tree = NULL;
+ c->index = 0;
free_commit_buffer(c);
free_commit_list(c->parents);
/* TODO: what about commit->util? */
+
+ c->object.parsed = 0;
}
const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
diff --git c/object.c w/object.c
index 9d5b10d5a20..8e29f63bf23 100644
--- c/object.c
+++ w/object.c
@@ -528,7 +528,7 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
else if (obj->type == OBJ_COMMIT)
release_commit_memory((struct commit*)obj);
else if (obj->type == OBJ_TAG)
- free_tag_buffer((struct tag*)obj);
+ release_tag_memory((struct tag*)obj);
}
FREE_AND_NULL(o->obj_hash);
diff --git c/tag.c w/tag.c
index 254352c30c6..7c12426b4ea 100644
--- c/tag.c
+++ w/tag.c
@@ -116,9 +116,12 @@ 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)
+void release_tag_memory(struct tag *t)
{
free(t->tag);
+ t->tagged = NULL;
+ t->object.parsed = 0;
+ t->date = 0;
}
int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)
diff --git c/tag.h w/tag.h
index b241fe67bc5..9057d76a506 100644
--- c/tag.h
+++ w/tag.h
@@ -15,7 +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 void release_tag_memory(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,alloc.c | 65 ++++++++++++++++++++++++++++++----------------- alloc.h | 19 ++++++++++++++ blame.c | 1 + blob.c | 1 + cache.h | 16 ------------ commit.c | 12 +++++++++ commit.h | 6 +++++ merge-recursive.c | 1 + object.c | 42 ++++++++++++++++++++++++++++-- object.h | 8 ++++++ tag.c | 9 +++++++ tag.h | 1 + tree.c | 1 + 13 files changed, 140 insertions(+), 42 deletions(-) create mode 100644 alloc.h
alloc.c +41 -24
@@ -4,8 +4,7 @@* Copyright (C) 2006 Linus Torvalds * * The standard malloc/free wastes too much space for objects, partly because -* it maintains all the allocation infrastructure (which isn't needed, since -* we never free an object descriptor anyway), but even more because it ends +* it maintains all the allocation infrastructure, but even more because it ends * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. */
@@ -15,6 +14,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #define BLOCKING 1024
@@ -30,8 +30,27 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs; +int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + 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) {void *ret;
@@ -39,60 +58,57 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)if (!s->nr) { s->nr =BLOCKING ; s->p =xmalloc (BLOCKING * node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = s->p; } s->nr--; s->count++; ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); +return ret; } -static struct alloc_state blob_state; -void *alloc_blob_node_the_repository (void ) +void *alloc_blob_node (struct repository *r) { -struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); +struct blob *b =alloc_node (r->parsed_objects->blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;return b; } -static struct alloc_state tree_state; -void *alloc_tree_node_the_repository (void ) +void *alloc_tree_node (struct repository *r) { -struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); +struct tree *t =alloc_node (r->parsed_objects->tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;return t; } -static struct alloc_state tag_state; -void *alloc_tag_node_the_repository (void ) +void *alloc_tag_node (struct repository *r) { -struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); +struct tag *t =alloc_node (r->parsed_objects->tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;return t; } -static struct alloc_state object_state; -void *alloc_object_node_the_repository (void ) +void *alloc_object_node (struct repository *r) { -struct object *obj =alloc_node (&object_state,sizeof (union any_object )); +struct object *obj =alloc_node (r->parsed_objects->object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;return obj; } -static struct alloc_state commit_state; - -unsigned int alloc_commit_index_the_repository (void ) +unsigned int alloc_commit_index (struct repository *r) { -static unsigned int count; -return count++; +return r->parsed_objects->commit_count++; } -void *alloc_commit_node_the_repository (void ) +void *alloc_commit_node (struct repository *r) { -struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); +struct commit *c =alloc_node (r->parsed_objects->commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (the_repository); + c->index =alloc_commit_index (r);return c; }
@@ -103,9 +119,10 @@ static void report(const char *name, unsigned int count, size_t size) }#define REPORT (name, type) \ - report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) + report(#name, r->parsed_objects->name##_state->count, \ + r->parsed_objects->name##_state->count * sizeof(type) >> 10) -void alloc_report_the_repository (void ) +void alloc_report (struct repository *r) { REPORT(blob, struct blob); REPORT(tree, struct tree);
alloc.h new file +19 -0
@@ -0,0 +1,19 @@ +#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); +void *alloc_tag_node (struct repository *r); +void *alloc_object_node (struct repository *r); +void alloc_report (struct repository *r); +unsigned int alloc_commit_index (struct repository *r); + +void *allocate_alloc_state (void ); +void clear_alloc_state (struct alloc_state *s); + +#endif
blame.c +1 -0
@@ -6,6 +6,7 @@#include "diffcore.h" #include "tag.h" #include "blame.h" +#include "alloc.h" void blame_origin_decref (struct blame_origin *o) {
blob.c +1 -0
@@ -1,6 +1,7 @@#include "cache.h" #include "blob.h" #include "repository.h" +#include "alloc.h" const char *blob_type ="blob" ;
cache.h +0 -16
@@ -1763,22 +1763,6 @@ extern const char *excludes_file;int decode_85 (char *dst,const char *line,int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes); -/* alloc.c */ -#define alloc_blob_node (r) alloc_blob_node_##r() -extern void *alloc_blob_node_the_repository (void ); -#define alloc_tree_node (r) alloc_tree_node_##r() -extern void *alloc_tree_node_the_repository (void ); -#define alloc_commit_node (r) alloc_commit_node_##r() -extern void *alloc_commit_node_the_repository (void ); -#define alloc_tag_node (r) alloc_tag_node_##r() -extern void *alloc_tag_node_the_repository (void ); -#define alloc_object_node (r) alloc_object_node_##r() -extern void *alloc_object_node_the_repository (void ); -#define alloc_report (r) alloc_report_##r() -extern void alloc_report_the_repository (void ); -#define alloc_commit_index (r) alloc_commit_index_##r() -extern unsigned int alloc_commit_index_the_repository (void ); -/* pkt-line.c */ void packet_trace_identity (const char *prog);
commit.c +12 -0
@@ -6,6 +6,7 @@#include "diff.h" #include "revision.h" #include "notes.h" +#include "alloc.h" #include "gpg-interface.h" #include "mergesort.h" #include "commit-slab.h"
@@ -296,6 +297,17 @@ void free_commit_buffer(struct commit *commit) } } +void release_commit_memory (struct commit *c) +{ + c->tree =NULL ; + c->index =0 ; +free_commit_buffer (c); +free_commit_list (c->parents); +/* TODO: what about commit->util? */ + + c->object.parsed =0 ; +} +const void *detach_commit_buffer (struct commit *commit,unsigned long *sizep) {struct commit_buffer *v =buffer_slab_peek (&buffer_slab, commit);
commit.h +6 -0
@@ -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.
merge-recursive.c +1 -0
@@ -15,6 +15,7 @@#include "diff.h" #include "diffcore.h" #include "tag.h" +#include "alloc.h" #include "unpack-trees.h" #include "string-list.h" #include "xdiff-interface.h"
object.c +40 -2
61 lines not on this page · raw message
object.h +8 -0
@@ -4,6 +4,14 @@struct parsed_object_pool {struct object **obj_hash;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; +unsigned commit_count; };struct parsed_object_pool *parsed_object_pool_new (void );
tag.c +9 -0
@@ -3,6 +3,7 @@#include "commit.h" #include "tree.h" #include "blob.h" +#include "alloc.h" #include "gpg-interface.h" const char *tag_type ="tag" ;
@@ -115,6 +116,14 @@ static timestamp_t parse_tag_date(const char *buf, const char *tail)return parse_timestamp (dateptr,NULL ,10 ); } +void release_tag_memory (struct tag *t) +{ +free (t->tag); + t->tagged =NULL ; + t->object.parsed =0 ; + t->date =0 ; +} +int parse_tag_buffer (struct tag *item,const void *data,unsigned long size) {struct object_id oid;
tag.h +1 -0
@@ -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 release_tag_memory (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,
tree.c +1 -0
@@ -5,6 +5,7 @@#include "blob.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "tree-walk.h" const char *tree_type ="tree" ;
Signature · 1 line
2.17.0.582.gccdcbd54c44.dirty
Re: [PATCH] alloc: allow arbitrary repositories for alloc functions
Quoted 18 lines · Stefan Beller
We have to convert all of the alloc functions at once, because
alloc_reportuses a funky macro for reporting. It is better for the sake of mechanical conversion to convert multiple functions at once rather than changing the structure of the reporting function.We record all memory allocation in
alloc.c, and free them inclear_alloc_state, which is called for all repositories exceptthe_repository.Signed-off-by Stefan Beller <[email protected]>Signed-off-by Junio C Hamano <[email protected]>---
Notes:
Eric, I have fixed s/relase/release/
Quoted 8 lines
Jonathan,
Quoted 4 lines
This might seem a bit bikesheddy, but I wouldn't call it
free_tag_buffer(), since what's being freed is not the buffer of the object itself, but just a string. If you want such a function, I would just call itrelease_tag_memory()to matchrelease_commit_memory().So you would replace the last commit with a patch like this?
Quite honestly, I do not see much difference either way between free and release. None of the resetting the .parsed bit, .index to 0, and tagged to NULL is about releasing or freeing resources. These operations are about de-initializing the object, which may well involve releasing resources directly associated with the object itself, but even more. The release_commit_memory() function may want to do something to various commit slabs that refer back to the commit, for example.
But as I said, I do not deeply care either way ;-)
Thanks, the patch looks sensible.
Quoted 499 lines
Thanks,
Stefandiff to what is currently queued: diff --git c/commit.c w/commit.c index 612ccf7b053..5eb4d2f08f8 100644 --- c/commit.c +++ w/commit.c @@ -299,9 +299,13 @@ void free_commit_buffer(struct commit *commit) void release_commit_memory(struct commit *c) { + c->tree = NULL; + c->index = 0; free_commit_buffer(c); free_commit_list(c->parents); /* TODO: what about commit->util? */ + + c->object.parsed = 0; } const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep) diff --git c/object.c w/object.c index 9d5b10d5a20..8e29f63bf23 100644 --- c/object.c +++ w/object.c @@ -528,7 +528,7 @@ void parsed_object_pool_clear(struct parsed_object_pool *o) else if (obj->type == OBJ_COMMIT) release_commit_memory((struct commit*)obj); else if (obj->type == OBJ_TAG) - free_tag_buffer((struct tag*)obj); + release_tag_memory((struct tag*)obj); } FREE_AND_NULL(o->obj_hash); diff --git c/tag.c w/tag.c index 254352c30c6..7c12426b4ea 100644 --- c/tag.c +++ w/tag.c @@ -116,9 +116,12 @@ 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) +void release_tag_memory(struct tag *t) { free(t->tag); + t->tagged = NULL; + t->object.parsed = 0; + t->date = 0; } int parse_tag_buffer(struct tag *item, const void *data, unsigned long size) diff --git c/tag.h w/tag.h index b241fe67bc5..9057d76a506 100644 --- c/tag.h +++ w/tag.h @@ -15,7 +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 void release_tag_memory(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,alloc.c | 65 ++++++++++++++++++++++++++++++----------------- alloc.h | 19 ++++++++++++++ blame.c | 1 + blob.c | 1 + cache.h | 16 ------------ commit.c | 12 +++++++++ commit.h | 6 +++++ merge-recursive.c | 1 + object.c | 42 ++++++++++++++++++++++++++++-- object.h | 8 ++++++ tag.c | 9 +++++++ tag.h | 1 + tree.c | 1 + 13 files changed, 140 insertions(+), 42 deletions(-) create mode 100644 alloc.halloc.c +41
-24@@ -4,8 +4,7 @@* Copyright (C) 2006 Linus Torvalds * * The standard malloc/free wastes too much space for objects, partly because -* it maintains all the allocation infrastructure (which isn't needed, since -* we never free an object descriptor anyway), but even more because it ends +* it maintains all the allocation infrastructure, but even more because it ends * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. */ @@ -15,6 +14,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #define BLOCKING 1024@@ -30,8 +30,27 @@ struct alloc_state {int count;/* total number of nodes allocated */ int nr;/* number of nodes left in current allocation */ void *p;/* first free node in current allocation */ + +/* bookkeeping of allocations */ +void **slabs; +int slab_nr, slab_alloc; }; +void *allocate_alloc_state (void ) +{ +return xcalloc (1 ,sizeof (struct alloc_state )); +} + +void clear_alloc_state (struct alloc_state *s) +{ +while (s->slab_nr >0 ) { + 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) {void *ret;@@ -39,60 +58,57 @@ static inline void *alloc_node(struct alloc_state *s, size_t node_size)if (!s->nr) { s->nr =BLOCKING ; s->p =xmalloc (BLOCKING * node_size); + +ALLOC_GROW (s->slabs, s->slab_nr +1 , s->slab_alloc); + s->slabs[s->slab_nr++] = s->p; } s->nr--; s->count++; ret = s->p; s->p = (char *)s->p + node_size;memset (ret,0 , node_size); +return ret; } -static struct alloc_state blob_state; -void *alloc_blob_node_the_repository (void ) +void *alloc_blob_node (struct repository *r) { -struct blob *b =alloc_node (&blob_state,sizeof (struct blob )); +struct blob *b =alloc_node (r->parsed_objects->blob_state,sizeof (struct blob )); b->object.type =OBJ_BLOB ;return b; } -static struct alloc_state tree_state; -void *alloc_tree_node_the_repository (void ) +void *alloc_tree_node (struct repository *r) { -struct tree *t =alloc_node (&tree_state,sizeof (struct tree )); +struct tree *t =alloc_node (r->parsed_objects->tree_state,sizeof (struct tree )); t->object.type =OBJ_TREE ;return t; } -static struct alloc_state tag_state; -void *alloc_tag_node_the_repository (void ) +void *alloc_tag_node (struct repository *r) { -struct tag *t =alloc_node (&tag_state,sizeof (struct tag )); +struct tag *t =alloc_node (r->parsed_objects->tag_state,sizeof (struct tag )); t->object.type =OBJ_TAG ;return t; } -static struct alloc_state object_state; -void *alloc_object_node_the_repository (void ) +void *alloc_object_node (struct repository *r) { -struct object *obj =alloc_node (&object_state,sizeof (union any_object )); +struct object *obj =alloc_node (r->parsed_objects->object_state,sizeof (union any_object )); obj->type =OBJ_NONE ;return obj; } -static struct alloc_state commit_state; - -unsigned int alloc_commit_index_the_repository (void ) +unsigned int alloc_commit_index (struct repository *r) { -static unsigned int count; -return count++; +return r->parsed_objects->commit_count++; } -void *alloc_commit_node_the_repository (void ) +void *alloc_commit_node (struct repository *r) { -struct commit *c =alloc_node (&commit_state,sizeof (struct commit )); +struct commit *c =alloc_node (r->parsed_objects->commit_state,sizeof (struct commit )); c->object.type =OBJ_COMMIT ; - c->index =alloc_commit_index (the_repository); + c->index =alloc_commit_index (r);return c; }@@ -103,9 +119,10 @@ static void report(const char *name, unsigned int count, size_t size) }#define REPORT (name, type) \ - report(#name, name##_state.count, name##_state.count * sizeof(type) >> 10) + report(#name, r->parsed_objects->name##_state->count, \ + r->parsed_objects->name##_state->count * sizeof(type) >> 10) -void alloc_report_the_repository (void ) +void alloc_report (struct repository *r) { REPORT(blob, struct blob); REPORT(tree, struct tree);alloc.h new file +19
-0@@ -0,0 +1,19 @@ +#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); +void *alloc_tag_node (struct repository *r); +void *alloc_object_node (struct repository *r); +void alloc_report (struct repository *r); +unsigned int alloc_commit_index (struct repository *r); + +void *allocate_alloc_state (void ); +void clear_alloc_state (struct alloc_state *s); + +#endif blame.c +1
-0@@ -6,6 +6,7 @@#include "diffcore.h" #include "tag.h" #include "blame.h" +#include "alloc.h" void blame_origin_decref (struct blame_origin *o) {blob.c +1
-0@@ -1,6 +1,7 @@#include "cache.h" #include "blob.h" #include "repository.h" +#include "alloc.h" const char *blob_type ="blob" ;cache.h +0
-16@@ -1763,22 +1763,6 @@ extern const char *excludes_file;int decode_85 (char *dst,const char *line,int linelen);void encode_85 (char *buf,const unsigned char *data,int bytes); -/* alloc.c */ -#define alloc_blob_node (r) alloc_blob_node_##r() -extern void *alloc_blob_node_the_repository (void ); -#define alloc_tree_node (r) alloc_tree_node_##r() -extern void *alloc_tree_node_the_repository (void ); -#define alloc_commit_node (r) alloc_commit_node_##r() -extern void *alloc_commit_node_the_repository (void ); -#define alloc_tag_node (r) alloc_tag_node_##r() -extern void *alloc_tag_node_the_repository (void ); -#define alloc_object_node (r) alloc_object_node_##r() -extern void *alloc_object_node_the_repository (void ); -#define alloc_report (r) alloc_report_##r() -extern void alloc_report_the_repository (void ); -#define alloc_commit_index (r) alloc_commit_index_##r() -extern unsigned int alloc_commit_index_the_repository (void ); -/* pkt-line.c */ void packet_trace_identity (const char *prog);commit.c +12
-0@@ -6,6 +6,7 @@#include "diff.h" #include "revision.h" #include "notes.h" +#include "alloc.h" #include "gpg-interface.h" #include "mergesort.h" #include "commit-slab.h" @@ -296,6 +297,17 @@ void free_commit_buffer(struct commit *commit) } } +void release_commit_memory (struct commit *c) +{ + c->tree =NULL ; + c->index =0 ; +free_commit_buffer (c); +free_commit_list (c->parents); +/* TODO: what about commit->util? */ + + c->object.parsed =0 ; +} +const void *detach_commit_buffer (struct commit *commit,unsigned long *sizep) {struct commit_buffer *v =buffer_slab_peek (&buffer_slab, commit);commit.h +6
-0@@ -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. merge-recursive.c +1
-0@@ -15,6 +15,7 @@#include "diff.h" #include "diffcore.h" #include "tag.h" +#include "alloc.h" #include "unpack-trees.h" #include "string-list.h" #include "xdiff-interface.h" object.c +40
-2@@ -5,6 +5,7 @@#include "tree.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "object-store.h" #include "packfile.h" @@ -455,6 +456,13 @@ 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 (); +return o; }@@ -501,9 +509,39 @@ void raw_object_store_clear(struct raw_object_store *o)void parsed_object_pool_clear (struct parsed_object_pool *o) {/* -* TOOD free objects in o->obj_hash. -* * As objects are allocated in slabs (see alloc.c), we do * not need to free each object, but each slab instead. +* +* Before doing so, we need to free any additional memory +* the objects may hold. */ +unsigned i; + +for (i =0 ; i < o->obj_hash_size; i++) { +struct object *obj = o->obj_hash[i]; + +if (!obj) +continue ; + +if (obj->type ==OBJ_TREE ) +free_tree_buffer ((struct tree *)obj); +else if (obj->type ==OBJ_COMMIT ) +release_commit_memory ((struct commit *)obj); +else if (obj->type ==OBJ_TAG ) +release_tag_memory ((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); +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); }object.h +8
-0@@ -4,6 +4,14 @@struct parsed_object_pool {struct object **obj_hash;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; +unsigned commit_count; };struct parsed_object_pool *parsed_object_pool_new (void );tag.c +9
-0@@ -3,6 +3,7 @@#include "commit.h" #include "tree.h" #include "blob.h" +#include "alloc.h" #include "gpg-interface.h" const char *tag_type ="tag" ;@@ -115,6 +116,14 @@ static timestamp_t parse_tag_date(const char *buf, const char *tail)return parse_timestamp (dateptr,NULL ,10 ); } +void release_tag_memory (struct tag *t) +{ +free (t->tag); + t->tagged =NULL ; + t->object.parsed =0 ; + t->date =0 ; +} +int parse_tag_buffer (struct tag *item,const void *data,unsigned long size) {struct object_id oid;tag.h +1
-0@@ -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 release_tag_memory (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,tree.c +1
-0@@ -5,6 +5,7 @@#include "blob.h" #include "commit.h" #include "tag.h" +#include "alloc.h" #include "tree-walk.h" const char *tree_type ="tree" ;