From 5df184241a6cfe88ac5ebcee9a3ad175abfca0cd Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Mon, 1 Apr 2013 19:38:23 +0200 Subject: [PATCH 01/25] lol this worked first try wtf --- src/cache.c | 187 ++++++++++++++++++++++++++--------------- src/cache.h | 33 ++++---- src/object.c | 54 +++++++----- src/odb.c | 36 ++++---- src/odb.h | 4 +- src/oidmap.h | 6 +- src/repository.c | 2 +- src/util.c | 8 -- tests-clar/core/opts.c | 11 --- 9 files changed, 193 insertions(+), 148 deletions(-) diff --git a/src/cache.c b/src/cache.c index e7f333577..6eb18dae5 100644 --- a/src/cache.c +++ b/src/cache.c @@ -11,100 +11,147 @@ #include "thread-utils.h" #include "util.h" #include "cache.h" +#include "odb.h" +#include "object.h" #include "git2/oid.h" -int git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_ptr) +GIT__USE_OIDMAP + +int git_cache_init(git_cache *cache) { - if (size < 8) - size = 8; - size = git__size_t_powerof2(size); - - cache->size_mask = size - 1; - cache->lru_count = 0; - cache->free_obj = free_ptr; - + cache->map = git_oidmap_alloc(); git_mutex_init(&cache->lock); - - cache->nodes = git__malloc(size * sizeof(git_cached_obj *)); - GITERR_CHECK_ALLOC(cache->nodes); - - memset(cache->nodes, 0x0, size * sizeof(git_cached_obj *)); return 0; } void git_cache_free(git_cache *cache) { - size_t i; - - for (i = 0; i < (cache->size_mask + 1); ++i) { - if (cache->nodes[i] != NULL) - git_cached_obj_decref(cache->nodes[i], cache->free_obj); - } - + git_oidmap_free(cache->map); git_mutex_free(&cache->lock); - git__free(cache->nodes); } -void *git_cache_get(git_cache *cache, const git_oid *oid) +static bool cache_should_store(git_cached_obj *entry) { - uint32_t hash; - git_cached_obj *node = NULL, *result = NULL; - - memcpy(&hash, oid->id, sizeof(hash)); - - if (git_mutex_lock(&cache->lock)) { - giterr_set(GITERR_THREAD, "unable to lock cache mutex"); - return NULL; - } - - { - node = cache->nodes[hash & cache->size_mask]; - - if (node != NULL && git_oid_cmp(&node->oid, oid) == 0) { - git_cached_obj_incref(node); - result = node; - } - } - git_mutex_unlock(&cache->lock); - - return result; + return true; } -void *git_cache_try_store(git_cache *cache, void *_entry) +static void *cache_get(git_cache *cache, const git_oid *oid, unsigned int flags) { - git_cached_obj *entry = _entry; - uint32_t hash; + khiter_t pos; + git_cached_obj *entry = NULL; - memcpy(&hash, &entry->oid, sizeof(uint32_t)); - - if (git_mutex_lock(&cache->lock)) { - giterr_set(GITERR_THREAD, "unable to lock cache mutex"); + if (git_mutex_lock(&cache->lock) < 0) return NULL; - } - { - git_cached_obj *node = cache->nodes[hash & cache->size_mask]; + pos = kh_get(oid, cache->map, oid); + if (pos != kh_end(cache->map)) { + entry = kh_val(cache->map, pos); - /* increase the refcount on this object, because - * the cache now owns it */ - git_cached_obj_incref(entry); - - if (node == NULL) { - cache->nodes[hash & cache->size_mask] = entry; - } else if (git_oid_cmp(&node->oid, &entry->oid) == 0) { - git_cached_obj_decref(entry, cache->free_obj); - entry = node; + if (flags && entry->flags != flags) { + entry = NULL; } else { - git_cached_obj_decref(node, cache->free_obj); - cache->nodes[hash & cache->size_mask] = entry; + git_cached_obj_incref(entry); } - - /* increase the refcount again, because we are - * returning it to the user */ - git_cached_obj_incref(entry); - } + git_mutex_unlock(&cache->lock); return entry; } + +static void *cache_store(git_cache *cache, git_cached_obj *entry) +{ + khiter_t pos; + + git_cached_obj_incref(entry); + + if (!cache_should_store(entry)) + return entry; + + if (git_mutex_lock(&cache->lock) < 0) + return entry; + + pos = kh_get(oid, cache->map, &entry->oid); + + /* not found */ + if (pos == kh_end(cache->map)) { + int rval; + + pos = kh_put(oid, cache->map, &entry->oid, &rval); + if (rval >= 0) { + kh_key(cache->map, pos) = &entry->oid; + kh_val(cache->map, pos) = entry; + git_cached_obj_incref(entry); + } + } + /* found */ + else { + git_cached_obj *stored_entry = kh_val(cache->map, pos); + + if (stored_entry->flags == entry->flags) { + git_cached_obj_decref(entry); + git_cached_obj_incref(stored_entry); + entry = stored_entry; + } else if (stored_entry->flags == GIT_CACHE_STORE_RAW && + entry->flags == GIT_CACHE_STORE_PARSED) { + git_cached_obj_decref(stored_entry); + git_cached_obj_incref(entry); + + kh_key(cache->map, pos) = &entry->oid; + kh_val(cache->map, pos) = entry; + } else { + /* NO OP */ + } + } + + git_mutex_unlock(&cache->lock); + return entry; +} + +void *git_cache_store_raw(git_cache *cache, git_odb_object *entry) +{ + entry->cached.flags = GIT_CACHE_STORE_RAW; + return cache_store(cache, (git_cached_obj *)entry); +} + +void *git_cache_store_parsed(git_cache *cache, git_object *entry) +{ + entry->cached.flags = GIT_CACHE_STORE_PARSED; + return cache_store(cache, (git_cached_obj *)entry); +} + +git_odb_object *git_cache_get_raw(git_cache *cache, const git_oid *oid) +{ + return cache_get(cache, oid, GIT_CACHE_STORE_RAW); +} + +git_object *git_cache_get_parsed(git_cache *cache, const git_oid *oid) +{ + return cache_get(cache, oid, GIT_CACHE_STORE_PARSED); +} + +void *git_cache_get_any(git_cache *cache, const git_oid *oid) +{ + return cache_get(cache, oid, GIT_CACHE_STORE_ANY); +} + +void git_cached_obj_decref(void *_obj) +{ + git_cached_obj *obj = _obj; + + if (git_atomic_dec(&obj->refcount) == 0) { + switch (obj->flags) { + case GIT_CACHE_STORE_RAW: + git_odb_object__free(_obj); + break; + + case GIT_CACHE_STORE_PARSED: + git_object__free(_obj); + break; + + default: + git__free(_obj); + break; + } + } +} diff --git a/src/cache.h b/src/cache.h index 7034ec268..f30af9c3e 100644 --- a/src/cache.h +++ b/src/cache.h @@ -12,30 +12,35 @@ #include "git2/odb.h" #include "thread-utils.h" +#include "oidmap.h" -#define GIT_DEFAULT_CACHE_SIZE 128 - -typedef void (*git_cached_obj_freeptr)(void *); +enum { + GIT_CACHE_STORE_ANY = 0, + GIT_CACHE_STORE_RAW = 1, + GIT_CACHE_STORE_PARSED = 2 +}; typedef struct { git_oid oid; git_atomic refcount; + uint32_t flags; } git_cached_obj; typedef struct { - git_cached_obj **nodes; + git_oidmap *map; git_mutex lock; - unsigned int lru_count; - size_t size_mask; - git_cached_obj_freeptr free_obj; } git_cache; -int git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_ptr); +int git_cache_init(git_cache *cache); void git_cache_free(git_cache *cache); -void *git_cache_try_store(git_cache *cache, void *entry); -void *git_cache_get(git_cache *cache, const git_oid *oid); +void *git_cache_store_raw(git_cache *cache, git_odb_object *entry); +void *git_cache_store_parsed(git_cache *cache, git_object *entry); + +git_odb_object *git_cache_get_raw(git_cache *cache, const git_oid *oid); +git_object *git_cache_get_parsed(git_cache *cache, const git_oid *oid); +void *git_cache_get_any(git_cache *cache, const git_oid *oid); GIT_INLINE(void) git_cached_obj_incref(void *_obj) { @@ -43,12 +48,6 @@ GIT_INLINE(void) git_cached_obj_incref(void *_obj) git_atomic_inc(&obj->refcount); } -GIT_INLINE(void) git_cached_obj_decref(void *_obj, git_cached_obj_freeptr free_obj) -{ - git_cached_obj *obj = _obj; - - if (git_atomic_dec(&obj->refcount) == 0) - free_obj(obj); -} +void git_cached_obj_decref(void *_obj); #endif diff --git a/src/object.c b/src/object.c index 80fe51152..5542ebc8e 100644 --- a/src/object.c +++ b/src/object.c @@ -121,12 +121,13 @@ int git_object__from_odb_object( break; } - if (error < 0) + if (error < 0) { git_object__free(object); - else - *object_out = git_cache_try_store(&repo->objects, object); + return error; + } - return error; + *object_out = git_cache_store_parsed(&repo->objects, object); + return 0; } int git_object_lookup_prefix( @@ -154,27 +155,38 @@ int git_object_lookup_prefix( len = GIT_OID_HEXSZ; if (len == GIT_OID_HEXSZ) { + git_cached_obj *cached = NULL; + /* We want to match the full id : we can first look up in the cache, * since there is no need to check for non ambiguousity */ - object = git_cache_get(&repo->objects, id); - if (object != NULL) { - if (type != GIT_OBJ_ANY && type != object->type) { - git_object_free(object); - giterr_set(GITERR_INVALID, "The requested type does not match the type in ODB"); - return GIT_ENOTFOUND; + cached = git_cache_get_any(&repo->objects, id); + if (cached != NULL) { + if (cached->flags == GIT_CACHE_STORE_PARSED) { + object = (git_object *)cached; + + if (type != GIT_OBJ_ANY && type != object->type) { + git_object_free(object); + giterr_set(GITERR_INVALID, + "The requested type does not match the type in ODB"); + return GIT_ENOTFOUND; + } + + *object_out = object; + return 0; + } else if (cached->flags == GIT_CACHE_STORE_RAW) { + odb_obj = (git_odb_object *)cached; + } else { + assert(!"Wrong caching type in the global object cache"); } - - *object_out = object; - return 0; + } else { + /* Object was not found in the cache, let's explore the backends. + * We could just use git_odb_read_unique_short_oid, + * it is the same cost for packed and loose object backends, + * but it may be much more costly for sqlite and hiredis. + */ + error = git_odb_read(&odb_obj, odb, id); } - - /* Object was not found in the cache, let's explore the backends. - * We could just use git_odb_read_unique_short_oid, - * it is the same cost for packed and loose object backends, - * but it may be much more costly for sqlite and hiredis. - */ - error = git_odb_read(&odb_obj, odb, id); } else { git_oid short_oid; @@ -245,7 +257,7 @@ void git_object_free(git_object *object) if (object == NULL) return; - git_cached_obj_decref((git_cached_obj *)object, git_object__free); + git_cached_obj_decref(object); } const git_oid *git_object_id(const git_object *obj) diff --git a/src/odb.c b/src/odb.c index d6ea8c29a..821fbd70c 100644 --- a/src/odb.c +++ b/src/odb.c @@ -14,6 +14,7 @@ #include "odb.h" #include "delta-apply.h" #include "filter.h" +#include "repository.h" #include "git2/odb_backend.h" #include "git2/oid.h" @@ -34,7 +35,15 @@ typedef struct ino_t disk_inode; } backend_internal; -size_t git_odb__cache_size = GIT_DEFAULT_CACHE_SIZE; +static git_cache *odb_cache(git_odb *odb) +{ + if (odb->rc.owner != NULL) { + git_repository *owner = odb->rc.owner; + return &owner->objects; + } + + return &odb->own_cache; +} static int load_alternates(git_odb *odb, const char *objects_dir, int alternate_depth); @@ -83,10 +92,8 @@ static git_odb_object *new_odb_object(const git_oid *oid, git_rawobj *source) return object; } -static void free_odb_object(void *o) +void git_odb_object__free(git_odb_object *object) { - git_odb_object *object = (git_odb_object *)o; - if (object != NULL) { git__free(object->raw.data); git__free(object); @@ -118,7 +125,7 @@ void git_odb_object_free(git_odb_object *object) if (object == NULL) return; - git_cached_obj_decref((git_cached_obj *)object, &free_odb_object); + git_cached_obj_decref(object); } int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type) @@ -355,9 +362,8 @@ int git_odb_new(git_odb **out) git_odb *db = git__calloc(1, sizeof(*db)); GITERR_CHECK_ALLOC(db); - if (git_cache_init(&db->cache, git_odb__cache_size, &free_odb_object) < 0 || - git_vector_init(&db->backends, 4, backend_sort_cmp) < 0) - { + if (git_cache_init(&db->own_cache) < 0 || + git_vector_init(&db->backends, 4, backend_sort_cmp) < 0) { git__free(db); return -1; } @@ -559,7 +565,7 @@ static void odb_free(git_odb *db) } git_vector_free(&db->backends); - git_cache_free(&db->cache); + git_cache_free(&db->own_cache); git__free(db); } @@ -580,7 +586,7 @@ int git_odb_exists(git_odb *db, const git_oid *id) assert(db && id); - if ((object = git_cache_get(&db->cache, id)) != NULL) { + if ((object = git_cache_get_raw(odb_cache(db), id)) != NULL) { git_odb_object_free(object); return (int)true; } @@ -630,7 +636,7 @@ int git_odb__read_header_or_object( assert(db && id && out && len_p && type_p); - if ((object = git_cache_get(&db->cache, id)) != NULL) { + if ((object = git_cache_get_raw(odb_cache(db), id)) != NULL) { *len_p = object->raw.len; *type_p = object->raw.type; *out = object; @@ -678,7 +684,7 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) return GIT_ENOTFOUND; } - *out = git_cache_get(&db->cache, id); + *out = git_cache_get_raw(odb_cache(db), id); if (*out != NULL) return 0; @@ -704,7 +710,7 @@ attempt_lookup: if (error && error != GIT_PASSTHROUGH) return error; - *out = git_cache_try_store(&db->cache, new_odb_object(id, &raw)); + *out = git_cache_store_raw(odb_cache(db), new_odb_object(id, &raw)); return 0; } @@ -727,7 +733,7 @@ int git_odb_read_prefix( len = GIT_OID_HEXSZ; if (len == GIT_OID_HEXSZ) { - *out = git_cache_get(&db->cache, short_id); + *out = git_cache_get_raw(odb_cache(db), short_id); if (*out != NULL) return 0; } @@ -768,7 +774,7 @@ attempt_lookup: if (!found) return git_odb__error_notfound("no match for prefix", short_id); - *out = git_cache_try_store(&db->cache, new_odb_object(&found_full_oid, &raw)); + *out = git_cache_store_raw(odb_cache(db), new_odb_object(&found_full_oid, &raw)); return 0; } diff --git a/src/odb.h b/src/odb.h index 7c018cc50..9abf594e8 100644 --- a/src/odb.h +++ b/src/odb.h @@ -36,9 +36,11 @@ struct git_odb_object { struct git_odb { git_refcount rc; git_vector backends; - git_cache cache; + git_cache own_cache; }; +void git_odb_object__free(git_odb_object *object); + /* * Hash a git_rawobj internally. * The `git_rawobj` is supposed to be previously initialized diff --git a/src/oidmap.h b/src/oidmap.h index 40274cd19..dfa951af3 100644 --- a/src/oidmap.h +++ b/src/oidmap.h @@ -21,10 +21,8 @@ typedef khash_t(oid) git_oidmap; GIT_INLINE(khint_t) hash_git_oid(const git_oid *oid) { - int i; - khint_t h = 0; - for (i = 0; i < 20; ++i) - h = (h << 5) - h + oid->id[i]; + khint_t h; + memcpy(&h, oid, sizeof(khint_t)); return h; } diff --git a/src/repository.c b/src/repository.c index a16f69da4..cda75b85f 100644 --- a/src/repository.c +++ b/src/repository.c @@ -119,7 +119,7 @@ static git_repository *repository_alloc(void) memset(repo, 0x0, sizeof(git_repository)); - if (git_cache_init(&repo->objects, GIT_DEFAULT_CACHE_SIZE, &git_object__free) < 0) { + if (git_cache_init(&repo->objects) < 0) { git__free(repo); return NULL; } diff --git a/src/util.c b/src/util.c index 8e83d298e..c4a8c786d 100644 --- a/src/util.c +++ b/src/util.c @@ -93,14 +93,6 @@ int git_libgit2_opts(int key, ...) if ((error = config_level_to_futils_dir(va_arg(ap, int))) >= 0) error = git_futils_dirs_set(error, va_arg(ap, const char *)); break; - - case GIT_OPT_GET_ODB_CACHE_SIZE: - *(va_arg(ap, size_t *)) = git_odb__cache_size; - break; - - case GIT_OPT_SET_ODB_CACHE_SIZE: - git_odb__cache_size = va_arg(ap, size_t); - break; } va_end(ap); diff --git a/tests-clar/core/opts.c b/tests-clar/core/opts.c index 907339d51..3173c648b 100644 --- a/tests-clar/core/opts.c +++ b/tests-clar/core/opts.c @@ -16,15 +16,4 @@ void test_core_opts__readwrite(void) git_libgit2_opts(GIT_OPT_GET_MWINDOW_SIZE, &new_val); cl_assert(new_val == old_val); - - git_libgit2_opts(GIT_OPT_GET_ODB_CACHE_SIZE, &old_val); - - cl_assert(old_val == GIT_DEFAULT_CACHE_SIZE); - - git_libgit2_opts(GIT_OPT_SET_ODB_CACHE_SIZE, (size_t)GIT_DEFAULT_CACHE_SIZE*2); - git_libgit2_opts(GIT_OPT_GET_ODB_CACHE_SIZE, &new_val); - - cl_assert(new_val == (GIT_DEFAULT_CACHE_SIZE*2)); - - git_libgit2_opts(GIT_OPT_GET_ODB_CACHE_SIZE, &old_val); } From 6b90e244de78640b751d0923c91c3868e12b8658 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Mon, 1 Apr 2013 19:53:49 +0200 Subject: [PATCH 02/25] Per-object filtering --- src/cache.c | 37 +++++++++++++++++++++++++++++++------ src/cache.h | 3 ++- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/cache.c b/src/cache.c index 6eb18dae5..a0925a188 100644 --- a/src/cache.c +++ b/src/cache.c @@ -17,8 +17,22 @@ GIT__USE_OIDMAP +bool git_cache__store_types[8] = { + false, /* GIT_OBJ__EXT1 */ + true, /* GIT_OBJ_COMMIT */ + true, /* GIT_OBJ_TREE */ + false, /* GIT_OBJ_BLOB */ + true, /* GIT_OBJ_TAG */ + false, /* GIT_OBJ__EXT2 */ + false, /* GIT_OBJ_OFS_DELTA */ + false /* GIT_OBJ_REF_DELTA */ +}; + +size_t git_cache__max_object_size = 4096; + int git_cache_init(git_cache *cache) { + cache->lru_count = 0; cache->map = git_oidmap_alloc(); git_mutex_init(&cache->lock); return 0; @@ -30,8 +44,14 @@ void git_cache_free(git_cache *cache) git_mutex_free(&cache->lock); } -static bool cache_should_store(git_cached_obj *entry) +static bool cache_should_store(git_otype object_type, size_t object_size) { + if (!git_cache__store_types[object_type]) + return false; + + if (object_size > git_cache__max_object_size) + return false; + return true; } @@ -63,11 +83,6 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) { khiter_t pos; - git_cached_obj_incref(entry); - - if (!cache_should_store(entry)) - return entry; - if (git_mutex_lock(&cache->lock) < 0) return entry; @@ -110,12 +125,22 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) void *git_cache_store_raw(git_cache *cache, git_odb_object *entry) { + git_cached_obj_incref(entry); + + if (!cache_should_store(entry->raw.type, entry->raw.len)) + return entry; + entry->cached.flags = GIT_CACHE_STORE_RAW; return cache_store(cache, (git_cached_obj *)entry); } void *git_cache_store_parsed(git_cache *cache, git_object *entry) { + git_cached_obj_incref(entry); + + if (!cache_should_store(entry->type, 0 /* TODO */)) + return entry; + entry->cached.flags = GIT_CACHE_STORE_PARSED; return cache_store(cache, (git_cached_obj *)entry); } diff --git a/src/cache.h b/src/cache.h index f30af9c3e..3461ff7d5 100644 --- a/src/cache.h +++ b/src/cache.h @@ -23,7 +23,8 @@ enum { typedef struct { git_oid oid; git_atomic refcount; - uint32_t flags; + uint16_t flags; + uint16_t lru; } git_cached_obj; typedef struct { From c4e91d4500bdd357fbf7378bc10648a482513fa6 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 3 Apr 2013 20:57:30 +0200 Subject: [PATCH 03/25] Random eviction --- src/cache.c | 19 ++++++++++++++++++- src/cache.h | 5 ++--- src/object.c | 1 + 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/cache.c b/src/cache.c index a0925a188..f49515ef3 100644 --- a/src/cache.c +++ b/src/cache.c @@ -32,7 +32,6 @@ size_t git_cache__max_object_size = 4096; int git_cache_init(git_cache *cache) { - cache->lru_count = 0; cache->map = git_oidmap_alloc(); git_mutex_init(&cache->lock); return 0; @@ -44,6 +43,24 @@ void git_cache_free(git_cache *cache) git_mutex_free(&cache->lock); } +static void cache_evict_entries(git_cache *cache, size_t evict) +{ + uint32_t seed = rand(); + + /* do not infinite loop if there's not enough entries to evict */ + if (evict > kh_size(cache->map)) + return; + + while (evict > 0) { + khiter_t pos = seed++ % kh_end(cache->map); + + if (kh_exist(cache->map, pos)) { + kh_del(oid, cache->map, pos); + evict--; + } + } +} + static bool cache_should_store(git_otype object_type, size_t object_size) { if (!git_cache__store_types[object_type]) diff --git a/src/cache.h b/src/cache.h index 3461ff7d5..f952830c5 100644 --- a/src/cache.h +++ b/src/cache.h @@ -23,14 +23,13 @@ enum { typedef struct { git_oid oid; git_atomic refcount; - uint16_t flags; - uint16_t lru; + uint32_t cache_size; + uint32_t flags; } git_cached_obj; typedef struct { git_oidmap *map; git_mutex lock; - unsigned int lru_count; } git_cache; int git_cache_init(git_cache *cache); diff --git a/src/object.c b/src/object.c index 5542ebc8e..54ceea33c 100644 --- a/src/object.c +++ b/src/object.c @@ -98,6 +98,7 @@ int git_object__from_odb_object( /* Initialize parent object */ git_oid_cpy(&object->cached.oid, &odb_obj->cached.oid); + object->cached.cache_size = (uint32_t)odb_obj->raw.len; object->repo = repo; switch (type) { From 8842c75f172ed94be4ad11521d4083e97d740785 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 3 Apr 2013 22:30:07 +0200 Subject: [PATCH 04/25] What has science done. --- src/blob.c | 10 +++++----- src/cache.c | 4 ++-- src/cache.h | 5 +++-- src/checkout.c | 4 ++-- src/commit.c | 2 +- src/commit_list.c | 15 +++++++++------ src/object.c | 9 +++++---- src/odb.c | 21 ++++++++++++--------- src/odb.h | 2 +- src/tag.c | 4 ++-- src/tree.c | 4 +++- tests-clar/diff/workdir.c | 3 ++- tests-clar/object/raw/write.c | 8 +++++++- tests-clar/odb/loose.c | 7 ++++++- tests-clar/odb/packed.c | 8 ++++---- tests-clar/odb/packed_one.c | 4 ++-- 16 files changed, 66 insertions(+), 44 deletions(-) diff --git a/src/blob.c b/src/blob.c index 11e1f4d77..7dce4f7ee 100644 --- a/src/blob.c +++ b/src/blob.c @@ -18,19 +18,19 @@ const void *git_blob_rawcontent(const git_blob *blob) { assert(blob); - return blob->odb_object->raw.data; + return blob->odb_object->buffer; } git_off_t git_blob_rawsize(const git_blob *blob) { assert(blob); - return (git_off_t)blob->odb_object->raw.len; + return (git_off_t)blob->odb_object->cached.size; } int git_blob__getbuf(git_buf *buffer, git_blob *blob) { return git_buf_set( - buffer, blob->odb_object->raw.data, blob->odb_object->raw.len); + buffer, blob->odb_object->buffer, blob->odb_object->cached.size); } void git_blob__free(git_blob *blob) @@ -315,8 +315,8 @@ int git_blob_is_binary(git_blob *blob) assert(blob); - content.ptr = blob->odb_object->raw.data; - content.size = min(blob->odb_object->raw.len, 4000); + content.ptr = blob->odb_object->buffer; + content.size = min(blob->odb_object->cached.size, 4000); return git_buf_text_is_binary(&content); } diff --git a/src/cache.c b/src/cache.c index f49515ef3..316007bdf 100644 --- a/src/cache.c +++ b/src/cache.c @@ -144,7 +144,7 @@ void *git_cache_store_raw(git_cache *cache, git_odb_object *entry) { git_cached_obj_incref(entry); - if (!cache_should_store(entry->raw.type, entry->raw.len)) + if (!cache_should_store(entry->cached.type, entry->cached.size)) return entry; entry->cached.flags = GIT_CACHE_STORE_RAW; @@ -155,7 +155,7 @@ void *git_cache_store_parsed(git_cache *cache, git_object *entry) { git_cached_obj_incref(entry); - if (!cache_should_store(entry->type, 0 /* TODO */)) + if (!cache_should_store(entry->cached.type, entry->cached.size)) return entry; entry->cached.flags = GIT_CACHE_STORE_PARSED; diff --git a/src/cache.h b/src/cache.h index f952830c5..3633f62a7 100644 --- a/src/cache.h +++ b/src/cache.h @@ -22,9 +22,10 @@ enum { typedef struct { git_oid oid; - git_atomic refcount; - uint32_t cache_size; + int32_t type; + size_t size; uint32_t flags; + git_atomic refcount; } git_cached_obj; typedef struct { diff --git a/src/checkout.c b/src/checkout.c index 81dc5e3da..bb2f90606 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -710,8 +710,8 @@ static int blob_content_to_file( git_vector filters = GIT_VECTOR_INIT; /* Create a fake git_buf from the blob raw data... */ - filtered.ptr = blob->odb_object->raw.data; - filtered.size = blob->odb_object->raw.len; + filtered.ptr = blob->odb_object->buffer; + filtered.size = blob->odb_object->cached.size; /* ... and make sure it doesn't get unexpectedly freed */ dont_free_filtered = true; diff --git a/src/commit.c b/src/commit.c index dd416920d..2057364b5 100644 --- a/src/commit.c +++ b/src/commit.c @@ -244,7 +244,7 @@ bad_buffer: int git_commit__parse(git_commit *commit, git_odb_object *obj) { assert(commit); - return git_commit__parse_buffer(commit, obj->raw.data, obj->raw.len); + return git_commit__parse_buffer(commit, obj->buffer, obj->cached.size); } #define GIT_COMMIT_GETTER(_rvalue, _name, _return) \ diff --git a/src/commit_list.c b/src/commit_list.c index 603dd754a..baabbbafb 100644 --- a/src/commit_list.c +++ b/src/commit_list.c @@ -100,12 +100,15 @@ git_commit_list_node *git_commit_list_pop(git_commit_list **stack) return item; } -static int commit_quick_parse(git_revwalk *walk, git_commit_list_node *commit, git_rawobj *raw) +static int commit_quick_parse( + git_revwalk *walk, + git_commit_list_node *commit, + uint8_t *buffer, + size_t buffer_len) { const size_t parent_len = strlen("parent ") + GIT_OID_HEXSZ + 1; - unsigned char *buffer = raw->data; - unsigned char *buffer_end = buffer + raw->len; - unsigned char *parents_start, *committer_start; + uint8_t *buffer_end = buffer + buffer_len; + uint8_t *parents_start, *committer_start; int i, parents = 0; int commit_time; @@ -182,11 +185,11 @@ int git_commit_list_parse(git_revwalk *walk, git_commit_list_node *commit) if ((error = git_odb_read(&obj, walk->odb, &commit->oid)) < 0) return error; - if (obj->raw.type != GIT_OBJ_COMMIT) { + if (obj->cached.type != GIT_OBJ_COMMIT) { giterr_set(GITERR_INVALID, "Object is no commit object"); error = -1; } else - error = commit_quick_parse(walk, commit, &obj->raw); + error = commit_quick_parse(walk, commit, obj->buffer, obj->cached.size); git_odb_object_free(obj); return error; diff --git a/src/object.c b/src/object.c index 54ceea33c..2667fcaf1 100644 --- a/src/object.c +++ b/src/object.c @@ -86,19 +86,20 @@ int git_object__from_odb_object( int error; git_object *object = NULL; - if (type != GIT_OBJ_ANY && type != odb_obj->raw.type) { - giterr_set(GITERR_INVALID, "The requested type does not match the type in the ODB"); + if (type != GIT_OBJ_ANY && type != odb_obj->cached.type) { + giterr_set(GITERR_INVALID, + "The requested type does not match the type in the ODB"); return GIT_ENOTFOUND; } - type = odb_obj->raw.type; + type = odb_obj->cached.type; if ((error = create_object(&object, type)) < 0) return error; /* Initialize parent object */ git_oid_cpy(&object->cached.oid, &odb_obj->cached.oid); - object->cached.cache_size = (uint32_t)odb_obj->raw.len; + object->cached.size = odb_obj->cached.size; object->repo = repo; switch (type) { diff --git a/src/odb.c b/src/odb.c index 821fbd70c..16a842aa8 100644 --- a/src/odb.c +++ b/src/odb.c @@ -65,6 +65,7 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj) if (!git_object_typeisloose(obj->type)) return -1; + if (!obj->data && obj->len != 0) return -1; @@ -87,7 +88,9 @@ static git_odb_object *new_odb_object(const git_oid *oid, git_rawobj *source) memset(object, 0x0, sizeof(git_odb_object)); git_oid_cpy(&object->cached.oid, oid); - memcpy(&object->raw, source, sizeof(git_rawobj)); + object->cached.size = source->len; + object->cached.type = source->type; + object->buffer = source->data; return object; } @@ -95,7 +98,7 @@ static git_odb_object *new_odb_object(const git_oid *oid, git_rawobj *source) void git_odb_object__free(git_odb_object *object) { if (object != NULL) { - git__free(object->raw.data); + git__free(object->buffer); git__free(object); } } @@ -107,17 +110,17 @@ const git_oid *git_odb_object_id(git_odb_object *object) const void *git_odb_object_data(git_odb_object *object) { - return object->raw.data; + return object->buffer; } size_t git_odb_object_size(git_odb_object *object) { - return object->raw.len; + return object->cached.size; } git_otype git_odb_object_type(git_odb_object *object) { - return object->raw.type; + return object->cached.type; } void git_odb_object_free(git_odb_object *object) @@ -637,8 +640,8 @@ int git_odb__read_header_or_object( assert(db && id && out && len_p && type_p); if ((object = git_cache_get_raw(odb_cache(db), id)) != NULL) { - *len_p = object->raw.len; - *type_p = object->raw.type; + *len_p = object->cached.size; + *type_p = object->cached.type; *out = object; return 0; } @@ -663,8 +666,8 @@ int git_odb__read_header_or_object( if ((error = git_odb_read(&object, db, id)) < 0) return error; /* error already set - pass along */ - *len_p = object->raw.len; - *type_p = object->raw.type; + *len_p = object->cached.size; + *type_p = object->cached.type; *out = object; return 0; diff --git a/src/odb.h b/src/odb.h index 9abf594e8..22c6e1668 100644 --- a/src/odb.h +++ b/src/odb.h @@ -29,7 +29,7 @@ typedef struct { /* EXPORT */ struct git_odb_object { git_cached_obj cached; - git_rawobj raw; + void *buffer; }; /* EXPORT */ diff --git a/src/tag.c b/src/tag.c index c82decbe3..3095d1287 100644 --- a/src/tag.c +++ b/src/tag.c @@ -324,7 +324,7 @@ int git_tag_create_frombuffer(git_oid *oid, git_repository *repo, const char *bu if (git_odb_read(&target_obj, odb, &tag.target) < 0) goto on_error; - if (tag.type != target_obj->raw.type) { + if (tag.type != target_obj->cached.type) { giterr_set(GITERR_TAG, "The type for the given target is invalid"); goto on_error; } @@ -397,7 +397,7 @@ int git_tag_delete(git_repository *repo, const char *tag_name) int git_tag__parse(git_tag *tag, git_odb_object *obj) { assert(tag); - return git_tag__parse_buffer(tag, obj->raw.data, obj->raw.len); + return git_tag__parse_buffer(tag, obj->buffer, obj->cached.size); } typedef struct { diff --git a/src/tree.c b/src/tree.c index d2db84055..6ffb07c69 100644 --- a/src/tree.c +++ b/src/tree.c @@ -419,7 +419,9 @@ static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buf int git_tree__parse(git_tree *tree, git_odb_object *obj) { assert(tree); - return tree_parse_buffer(tree, (char *)obj->raw.data, (char *)obj->raw.data + obj->raw.len); + return tree_parse_buffer(tree, + (char *)obj->buffer, + (char *)obj->buffer + obj->cached.size); } static size_t find_next_dir(const char *dirname, git_index *index, size_t start) diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 9d92d8d60..435bd4f2c 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -908,7 +908,6 @@ void test_diff_workdir__can_diff_empty_file(void) /* baseline - make sure there are no outstanding diffs */ cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, tree, &opts)); - git_tree_free(tree); cl_assert_equal_i(2, (int)git_diff_num_deltas(diff)); git_diff_list_free(diff); @@ -935,6 +934,8 @@ void test_diff_workdir__can_diff_empty_file(void) cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 1)); git_diff_patch_free(patch); git_diff_list_free(diff); + + git_tree_free(tree); } void test_diff_workdir__to_index_issue_1397(void) diff --git a/tests-clar/object/raw/write.c b/tests-clar/object/raw/write.c index 60aa31f6a..9709c0302 100644 --- a/tests-clar/object/raw/write.c +++ b/tests-clar/object/raw/write.c @@ -63,6 +63,7 @@ void test_body(object_data *d, git_rawobj *o) git_odb *db; git_oid id1, id2; git_odb_object *obj; + git_rawobj tmp; make_odb_dir(); cl_git_pass(git_odb_open(&db, odb_dir)); @@ -73,7 +74,12 @@ void test_body(object_data *d, git_rawobj *o) check_object_files(d); cl_git_pass(git_odb_read(&obj, db, &id1)); - cmp_objects(&obj->raw, o); + + tmp.data = obj->buffer; + tmp.len = obj->cached.size; + tmp.type = obj->cached.type; + + cmp_objects(&tmp, o); git_odb_object_free(obj); git_odb_free(db); diff --git a/tests-clar/odb/loose.c b/tests-clar/odb/loose.c index f95dc28d4..9539bb24c 100644 --- a/tests-clar/odb/loose.c +++ b/tests-clar/odb/loose.c @@ -30,6 +30,7 @@ static void test_read_object(object_data *data) git_oid id; git_odb_object *obj; git_odb *odb; + git_rawobj tmp; write_object_files(data); @@ -37,7 +38,11 @@ static void test_read_object(object_data *data) cl_git_pass(git_oid_fromstr(&id, data->id)); cl_git_pass(git_odb_read(&obj, odb, &id)); - cmp_objects((git_rawobj *)&obj->raw, data); + tmp.data = obj->buffer; + tmp.len = obj->cached.size; + tmp.type = obj->cached.type; + + cmp_objects(&tmp, data); git_odb_object_free(obj); git_odb_free(odb); diff --git a/tests-clar/odb/packed.c b/tests-clar/odb/packed.c index 90e9f3abd..b4f549b58 100644 --- a/tests-clar/odb/packed.c +++ b/tests-clar/odb/packed.c @@ -46,8 +46,8 @@ void test_odb_packed__read_header_0(void) cl_git_pass(git_odb_read(&obj, _odb, &id)); cl_git_pass(git_odb_read_header(&len, &type, _odb, &id)); - cl_assert(obj->raw.len == len); - cl_assert(obj->raw.type == type); + cl_assert(obj->cached.size == len); + cl_assert(obj->cached.type == type); git_odb_object_free(obj); } @@ -70,8 +70,8 @@ void test_odb_packed__read_header_1(void) cl_git_pass(git_odb_read(&obj, _odb, &id)); cl_git_pass(git_odb_read_header(&len, &type, _odb, &id)); - cl_assert(obj->raw.len == len); - cl_assert(obj->raw.type == type); + cl_assert(obj->cached.size == len); + cl_assert(obj->cached.type == type); git_odb_object_free(obj); } diff --git a/tests-clar/odb/packed_one.c b/tests-clar/odb/packed_one.c index 4f9bde9ed..0c6ed387b 100644 --- a/tests-clar/odb/packed_one.c +++ b/tests-clar/odb/packed_one.c @@ -52,8 +52,8 @@ void test_odb_packed_one__read_header_0(void) cl_git_pass(git_odb_read(&obj, _odb, &id)); cl_git_pass(git_odb_read_header(&len, &type, _odb, &id)); - cl_assert(obj->raw.len == len); - cl_assert(obj->raw.type == type); + cl_assert(obj->cached.size == len); + cl_assert(obj->cached.type == type); git_odb_object_free(obj); } From cf7850a4f70a1153ed640744750391d99000d546 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 3 Apr 2013 23:09:54 +0200 Subject: [PATCH 05/25] Duplicated type object --- src/cache.h | 4 ++-- src/object.c | 15 ++++++--------- src/object.h | 1 - 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/cache.h b/src/cache.h index 3633f62a7..d57f244dd 100644 --- a/src/cache.h +++ b/src/cache.h @@ -22,9 +22,9 @@ enum { typedef struct { git_oid oid; - int32_t type; + int16_t type; + uint16_t flags; size_t size; - uint32_t flags; git_atomic refcount; } git_cached_obj; diff --git a/src/object.c b/src/object.c index 2667fcaf1..80b765ef9 100644 --- a/src/object.c +++ b/src/object.c @@ -71,8 +71,6 @@ static int create_object(git_object **object_out, git_otype type) return -1; } - object->type = type; - *object_out = object; return 0; } @@ -92,17 +90,16 @@ int git_object__from_odb_object( return GIT_ENOTFOUND; } - type = odb_obj->cached.type; - - if ((error = create_object(&object, type)) < 0) + if ((error = create_object(&object, odb_obj->cached.type)) < 0) return error; /* Initialize parent object */ git_oid_cpy(&object->cached.oid, &odb_obj->cached.oid); object->cached.size = odb_obj->cached.size; + object->cached.type = odb_obj->cached.type; object->repo = repo; - switch (type) { + switch (object->cached.type) { case GIT_OBJ_COMMIT: error = git_commit__parse((git_commit *)object, odb_obj); break; @@ -167,7 +164,7 @@ int git_object_lookup_prefix( if (cached->flags == GIT_CACHE_STORE_PARSED) { object = (git_object *)cached; - if (type != GIT_OBJ_ANY && type != object->type) { + if (type != GIT_OBJ_ANY && type != object->cached.type) { git_object_free(object); giterr_set(GITERR_INVALID, "The requested type does not match the type in ODB"); @@ -231,7 +228,7 @@ void git_object__free(void *_obj) assert(object); - switch (object->type) { + switch (object->cached.type) { case GIT_OBJ_COMMIT: git_commit__free((git_commit *)object); break; @@ -271,7 +268,7 @@ const git_oid *git_object_id(const git_object *obj) git_otype git_object_type(const git_object *obj) { assert(obj); - return obj->type; + return obj->cached.type; } git_repository *git_object_owner(const git_object *obj) diff --git a/src/object.h b/src/object.h index c1e50593c..d187c55b7 100644 --- a/src/object.h +++ b/src/object.h @@ -11,7 +11,6 @@ struct git_object { git_cached_obj cached; git_repository *repo; - git_otype type; }; /* fully free the object; internal method, DO NOT EXPORT */ From 064236ca45067c9a7189e0d30790b8f3541b91ad Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 3 Apr 2013 23:39:42 +0200 Subject: [PATCH 06/25] Per-object max size --- src/cache.c | 56 ++++++++++++++++++++++++++--------------------------- src/cache.h | 1 + 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/cache.c b/src/cache.c index 316007bdf..c6e983597 100644 --- a/src/cache.c +++ b/src/cache.c @@ -17,21 +17,20 @@ GIT__USE_OIDMAP -bool git_cache__store_types[8] = { - false, /* GIT_OBJ__EXT1 */ - true, /* GIT_OBJ_COMMIT */ - true, /* GIT_OBJ_TREE */ - false, /* GIT_OBJ_BLOB */ - true, /* GIT_OBJ_TAG */ - false, /* GIT_OBJ__EXT2 */ - false, /* GIT_OBJ_OFS_DELTA */ - false /* GIT_OBJ_REF_DELTA */ +size_t git_cache__max_object_size[8] = { + 0, /* GIT_OBJ__EXT1 */ + 4096, /* GIT_OBJ_COMMIT */ + 4096, /* GIT_OBJ_TREE */ + 0, /* GIT_OBJ_BLOB */ + 4096, /* GIT_OBJ_TAG */ + 0, /* GIT_OBJ__EXT2 */ + 0, /* GIT_OBJ_OFS_DELTA */ + 0 /* GIT_OBJ_REF_DELTA */ }; -size_t git_cache__max_object_size = 4096; - int git_cache_init(git_cache *cache) { + cache->used_memory = 0; cache->map = git_oidmap_alloc(); git_mutex_init(&cache->lock); return 0; @@ -43,30 +42,35 @@ void git_cache_free(git_cache *cache) git_mutex_free(&cache->lock); } -static void cache_evict_entries(git_cache *cache, size_t evict) +/* Call with lock, yo */ +static void cache_evict_entries(git_cache *cache, size_t evict_count) { uint32_t seed = rand(); /* do not infinite loop if there's not enough entries to evict */ - if (evict > kh_size(cache->map)) + if (evict_count > kh_size(cache->map)) return; - while (evict > 0) { + while (evict_count > 0) { khiter_t pos = seed++ % kh_end(cache->map); if (kh_exist(cache->map, pos)) { + git_cached_obj *evict = kh_val(cache->map, pos); + + evict_count--; + cache->used_memory -= evict->size; + git_cached_obj_decref(evict); + kh_del(oid, cache->map, pos); - evict--; } } } static bool cache_should_store(git_otype object_type, size_t object_size) { - if (!git_cache__store_types[object_type]) - return false; + size_t max_size = git_cache__max_object_size[object_type]; - if (object_size > git_cache__max_object_size) + if (max_size == 0 || object_size > max_size) return false; return true; @@ -100,6 +104,11 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) { khiter_t pos; + git_cached_obj_incref(entry); + + if (!cache_should_store(entry->type, entry->size)) + return entry; + if (git_mutex_lock(&cache->lock) < 0) return entry; @@ -114,6 +123,7 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) kh_key(cache->map, pos) = &entry->oid; kh_val(cache->map, pos) = entry; git_cached_obj_incref(entry); + cache->used_memory += entry->size; } } /* found */ @@ -142,22 +152,12 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) void *git_cache_store_raw(git_cache *cache, git_odb_object *entry) { - git_cached_obj_incref(entry); - - if (!cache_should_store(entry->cached.type, entry->cached.size)) - return entry; - entry->cached.flags = GIT_CACHE_STORE_RAW; return cache_store(cache, (git_cached_obj *)entry); } void *git_cache_store_parsed(git_cache *cache, git_object *entry) { - git_cached_obj_incref(entry); - - if (!cache_should_store(entry->cached.type, entry->cached.size)) - return entry; - entry->cached.flags = GIT_CACHE_STORE_PARSED; return cache_store(cache, (git_cached_obj *)entry); } diff --git a/src/cache.h b/src/cache.h index d57f244dd..65a6e5766 100644 --- a/src/cache.h +++ b/src/cache.h @@ -31,6 +31,7 @@ typedef struct { typedef struct { git_oidmap *map; git_mutex lock; + size_t used_memory; } git_cache; int git_cache_init(git_cache *cache); From d9d423e4215ac3cc17def7b1a353d03031b811f8 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 3 Apr 2013 23:53:32 +0200 Subject: [PATCH 07/25] Some stats --- src/cache.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/cache.c b/src/cache.c index c6e983597..f3ab8a684 100644 --- a/src/cache.c +++ b/src/cache.c @@ -28,6 +28,27 @@ size_t git_cache__max_object_size[8] = { 0 /* GIT_OBJ_REF_DELTA */ }; +void git_cache_dump_stats(git_cache *cache) +{ + git_cached_obj *object; + + if (kh_size(cache->map) == 0) + return; + + printf("Cache %p: %d items cached, %d bytes\n", + cache, kh_size(cache->map), (int)cache->used_memory); + + kh_foreach_value(cache->map, object, { + char oid_str[9]; + printf(" %s%c %s (%d)\n", + git_object_type2string(object->type), + object->flags == GIT_CACHE_STORE_PARSED ? '*' : ' ', + git_oid_tostr(oid_str, sizeof(oid_str), &object->oid), + (int)object->size + ); + }); +} + int git_cache_init(git_cache *cache) { cache->used_memory = 0; From e16e268457ab4ab8a4edc8153deca2c8c22dc757 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 4 Apr 2013 02:09:32 +0200 Subject: [PATCH 08/25] No longer needed --- include/git2/common.h | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index 5318e66b7..b8c3e42ce 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -131,8 +131,6 @@ enum { GIT_OPT_SET_MWINDOW_MAPPED_LIMIT, GIT_OPT_GET_SEARCH_PATH, GIT_OPT_SET_SEARCH_PATH, - GIT_OPT_GET_ODB_CACHE_SIZE, - GIT_OPT_SET_ODB_CACHE_SIZE, }; /** @@ -169,15 +167,6 @@ enum { * - `level` must be GIT_CONFIG_LEVEL_SYSTEM, GIT_CONFIG_LEVEL_GLOBAL, * or GIT_CONFIG_LEVEL_XDG. * - * opts(GIT_OPT_GET_ODB_CACHE_SIZE): - * Get the size of the libgit2 odb cache. - * - * opts(GIT_OPT_SET_ODB_CACHE_SIZE): - * Set the size of the of the libgit2 odb cache. This needs - * to be done before git_repository_open is called, since - * git_repository_open initializes the odb layer. Defaults - * to 128. - * * @param option Option key * @param ... value to set the option * @return 0 on success, <0 on failure From e183e375b83044d7852b8253553c4f782d73c140 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 5 Apr 2013 22:38:14 +0200 Subject: [PATCH 09/25] Clear the cache when there are too many items to expire --- src/cache.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/cache.c b/src/cache.c index f3ab8a684..9f3290f01 100644 --- a/src/cache.c +++ b/src/cache.c @@ -63,14 +63,33 @@ void git_cache_free(git_cache *cache) git_mutex_free(&cache->lock); } +void git_cache_clear(git_cache *cache) +{ + git_cached_obj *evict = NULL; + + if (git_mutex_lock(&cache->lock) < 0) + return; + + kh_foreach_value(cache->map, evict, { + git_cached_obj_decref(evict); + }); + + kh_clear(oid, cache->map); + cache->used_memory = 0; + + git_mutex_unlock(&cache->lock); +} + /* Call with lock, yo */ static void cache_evict_entries(git_cache *cache, size_t evict_count) { uint32_t seed = rand(); /* do not infinite loop if there's not enough entries to evict */ - if (evict_count > kh_size(cache->map)) + if (evict_count > kh_size(cache->map)) { + git_cache_clear(cache); return; + } while (evict_count > 0) { khiter_t pos = seed++ % kh_end(cache->map); From ee12272d170d6a9d60f13d6de6129f56bfb2fbf6 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 5 Apr 2013 22:48:39 +0200 Subject: [PATCH 10/25] Global option setters --- include/git2/common.h | 2 ++ src/cache.c | 10 ++++------ src/cache.h | 3 +++ src/util.c | 11 +++++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index b8c3e42ce..80d83d345 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -131,6 +131,8 @@ enum { GIT_OPT_SET_MWINDOW_MAPPED_LIMIT, GIT_OPT_GET_SEARCH_PATH, GIT_OPT_SET_SEARCH_PATH, + GIT_OPT_SET_CACHE_LIMIT, + GIT_OPT_ENABLE_CACHING }; /** diff --git a/src/cache.c b/src/cache.c index 9f3290f01..f8cddeaca 100644 --- a/src/cache.c +++ b/src/cache.c @@ -17,6 +17,8 @@ GIT__USE_OIDMAP +bool git_cache__enabled = true; + size_t git_cache__max_object_size[8] = { 0, /* GIT_OBJ__EXT1 */ 4096, /* GIT_OBJ_COMMIT */ @@ -109,11 +111,7 @@ static void cache_evict_entries(git_cache *cache, size_t evict_count) static bool cache_should_store(git_otype object_type, size_t object_size) { size_t max_size = git_cache__max_object_size[object_type]; - - if (max_size == 0 || object_size > max_size) - return false; - - return true; + return git_cache__enabled && object_size < max_size; } static void *cache_get(git_cache *cache, const git_oid *oid, unsigned int flags) @@ -121,7 +119,7 @@ static void *cache_get(git_cache *cache, const git_oid *oid, unsigned int flags) khiter_t pos; git_cached_obj *entry = NULL; - if (git_mutex_lock(&cache->lock) < 0) + if (!git_cache__enabled || git_mutex_lock(&cache->lock) < 0) return NULL; pos = kh_get(oid, cache->map, oid); diff --git a/src/cache.h b/src/cache.h index 65a6e5766..8b2aa1f79 100644 --- a/src/cache.h +++ b/src/cache.h @@ -20,6 +20,9 @@ enum { GIT_CACHE_STORE_PARSED = 2 }; +extern bool git_cache__enabled; +extern size_t git_cache__max_object_size[8]; + typedef struct { git_oid oid; int16_t type; diff --git a/src/util.c b/src/util.c index c4a8c786d..0b5fbdc5a 100644 --- a/src/util.c +++ b/src/util.c @@ -11,6 +11,7 @@ #include #include "posix.h" #include "fileops.h" +#include "cache.h" #ifdef _MSC_VER # include @@ -93,6 +94,16 @@ int git_libgit2_opts(int key, ...) if ((error = config_level_to_futils_dir(va_arg(ap, int))) >= 0) error = git_futils_dirs_set(error, va_arg(ap, const char *)); break; + + case GIT_OPT_SET_CACHE_LIMIT: { + git_otype type = (git_otype)va_arg(ap, int); + git_cache__max_object_size[type] = va_arg(ap, size_t); + break; + } + + case GIT_OPT_ENABLE_CACHING: + git_cache__enabled = va_arg(ap, int); + break; } va_end(ap); From badd85a61354ef7b62c5f8e53d740738e5ef1e57 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 10 Apr 2013 17:10:17 -0700 Subject: [PATCH 11/25] Use git_odb_object_data/_size whereever possible This uses the odb object accessors so we can change the internals more easily... --- src/blob.c | 8 +++++--- src/checkout.c | 4 ++-- src/commit.c | 3 ++- src/commit_list.c | 13 ++++++++----- src/tag.c | 3 ++- src/tree.c | 14 +++++++++----- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/blob.c b/src/blob.c index 7dce4f7ee..732b0f3de 100644 --- a/src/blob.c +++ b/src/blob.c @@ -18,19 +18,21 @@ const void *git_blob_rawcontent(const git_blob *blob) { assert(blob); - return blob->odb_object->buffer; + return git_odb_object_data(blob->odb_object); } git_off_t git_blob_rawsize(const git_blob *blob) { assert(blob); - return (git_off_t)blob->odb_object->cached.size; + return (git_off_t)git_odb_object_size(blob->odb_object); } int git_blob__getbuf(git_buf *buffer, git_blob *blob) { return git_buf_set( - buffer, blob->odb_object->buffer, blob->odb_object->cached.size); + buffer, + git_odb_object_data(blob->odb_object), + git_odb_object_size(blob->odb_object)); } void git_blob__free(git_blob *blob) diff --git a/src/checkout.c b/src/checkout.c index bb2f90606..62a73d6d0 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -710,8 +710,8 @@ static int blob_content_to_file( git_vector filters = GIT_VECTOR_INIT; /* Create a fake git_buf from the blob raw data... */ - filtered.ptr = blob->odb_object->buffer; - filtered.size = blob->odb_object->cached.size; + filtered.ptr = (void *)git_blob_rawcontent(blob); + filtered.size = (size_t)git_blob_rawsize(blob); /* ... and make sure it doesn't get unexpectedly freed */ dont_free_filtered = true; diff --git a/src/commit.c b/src/commit.c index 2057364b5..2cee44cd2 100644 --- a/src/commit.c +++ b/src/commit.c @@ -244,7 +244,8 @@ bad_buffer: int git_commit__parse(git_commit *commit, git_odb_object *obj) { assert(commit); - return git_commit__parse_buffer(commit, obj->buffer, obj->cached.size); + return git_commit__parse_buffer( + commit, git_odb_object_data(obj), git_odb_object_size(obj)); } #define GIT_COMMIT_GETTER(_rvalue, _name, _return) \ diff --git a/src/commit_list.c b/src/commit_list.c index baabbbafb..bd5b5201a 100644 --- a/src/commit_list.c +++ b/src/commit_list.c @@ -103,12 +103,12 @@ git_commit_list_node *git_commit_list_pop(git_commit_list **stack) static int commit_quick_parse( git_revwalk *walk, git_commit_list_node *commit, - uint8_t *buffer, + const uint8_t *buffer, size_t buffer_len) { const size_t parent_len = strlen("parent ") + GIT_OID_HEXSZ + 1; - uint8_t *buffer_end = buffer + buffer_len; - uint8_t *parents_start, *committer_start; + const uint8_t *buffer_end = buffer + buffer_len; + const uint8_t *parents_start, *committer_start; int i, parents = 0; int commit_time; @@ -127,7 +127,7 @@ static int commit_quick_parse( for (i = 0; i < parents; ++i) { git_oid oid; - if (git_oid_fromstr(&oid, (char *)buffer + strlen("parent ")) < 0) + if (git_oid_fromstr(&oid, (const char *)buffer + strlen("parent ")) < 0) return -1; commit->parents[i] = git_revwalk__commit_lookup(walk, &oid); @@ -189,7 +189,10 @@ int git_commit_list_parse(git_revwalk *walk, git_commit_list_node *commit) giterr_set(GITERR_INVALID, "Object is no commit object"); error = -1; } else - error = commit_quick_parse(walk, commit, obj->buffer, obj->cached.size); + error = commit_quick_parse( + walk, commit, + (const uint8_t *)git_odb_object_data(obj), + git_odb_object_size(obj)); git_odb_object_free(obj); return error; diff --git a/src/tag.c b/src/tag.c index 3095d1287..b76895d0c 100644 --- a/src/tag.c +++ b/src/tag.c @@ -397,7 +397,8 @@ int git_tag_delete(git_repository *repo, const char *tag_name) int git_tag__parse(git_tag *tag, git_odb_object *obj) { assert(tag); - return git_tag__parse_buffer(tag, obj->buffer, obj->cached.size); + return git_tag__parse_buffer( + tag, git_odb_object_data(obj), git_odb_object_size(obj)); } typedef struct { diff --git a/src/tree.c b/src/tree.c index 6ffb07c69..cc43b920c 100644 --- a/src/tree.c +++ b/src/tree.c @@ -371,7 +371,8 @@ static int tree_error(const char *str, const char *path) return -1; } -static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buffer_end) +static int tree_parse_buffer( + git_tree *tree, const char *buffer, const char *buffer_end) { if (git_vector_init(&tree->entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) return -1; @@ -418,10 +419,13 @@ static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buf int git_tree__parse(git_tree *tree, git_odb_object *obj) { - assert(tree); - return tree_parse_buffer(tree, - (char *)obj->buffer, - (char *)obj->buffer + obj->cached.size); + const char *buf; + + assert(tree && obj); + + buf = (const char *)git_odb_object_data(obj); + + return tree_parse_buffer(tree, buf, buf + git_odb_object_size(obj)); } static size_t find_next_dir(const char *dirname, git_index *index, size_t start) From b12b72ea82776bbbd4296eeac1376055b0487edf Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 12 Apr 2013 12:44:51 -0700 Subject: [PATCH 12/25] Add range checking around cache opts Add a git_cache_set_max_object_size method that does more checking around setting the max object size. Also add a git_cache_size to read the number of objects currently in the cache. This makes it easier to write tests. --- src/cache.c | 23 +++++++++++++++++------ src/cache.h | 12 +++++++++--- src/util.c | 8 +++++--- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/cache.c b/src/cache.c index f8cddeaca..263f736fa 100644 --- a/src/cache.c +++ b/src/cache.c @@ -19,17 +19,28 @@ GIT__USE_OIDMAP bool git_cache__enabled = true; -size_t git_cache__max_object_size[8] = { - 0, /* GIT_OBJ__EXT1 */ +static size_t git_cache__max_object_size[8] = { + 0, /* GIT_OBJ__EXT1 */ 4096, /* GIT_OBJ_COMMIT */ 4096, /* GIT_OBJ_TREE */ - 0, /* GIT_OBJ_BLOB */ + 0, /* GIT_OBJ_BLOB */ 4096, /* GIT_OBJ_TAG */ - 0, /* GIT_OBJ__EXT2 */ - 0, /* GIT_OBJ_OFS_DELTA */ - 0 /* GIT_OBJ_REF_DELTA */ + 0, /* GIT_OBJ__EXT2 */ + 0, /* GIT_OBJ_OFS_DELTA */ + 0 /* GIT_OBJ_REF_DELTA */ }; +int git_cache_set_max_object_size(git_otype type, size_t size) +{ + if (type < 0 || (size_t)type >= ARRAY_SIZE(git_cache__max_object_size)) { + giterr_set(GITERR_INVALID, "type out of range"); + return -1; + } + + git_cache__max_object_size[type] = size; + return 0; +} + void git_cache_dump_stats(git_cache *cache) { git_cached_obj *object; diff --git a/src/cache.h b/src/cache.h index 8b2aa1f79..13b630e89 100644 --- a/src/cache.h +++ b/src/cache.h @@ -20,9 +20,6 @@ enum { GIT_CACHE_STORE_PARSED = 2 }; -extern bool git_cache__enabled; -extern size_t git_cache__max_object_size[8]; - typedef struct { git_oid oid; int16_t type; @@ -37,6 +34,10 @@ typedef struct { size_t used_memory; } git_cache; +extern bool git_cache__enabled; + +int git_cache_set_max_object_size(git_otype type, size_t size); + int git_cache_init(git_cache *cache); void git_cache_free(git_cache *cache); @@ -47,6 +48,11 @@ git_odb_object *git_cache_get_raw(git_cache *cache, const git_oid *oid); git_object *git_cache_get_parsed(git_cache *cache, const git_oid *oid); void *git_cache_get_any(git_cache *cache, const git_oid *oid); +GIT_INLINE(size_t) git_cache_size(git_cache *cache) +{ + return (size_t)kh_size(cache->map); +} + GIT_INLINE(void) git_cached_obj_incref(void *_obj) { git_cached_obj *obj = _obj; diff --git a/src/util.c b/src/util.c index 0b5fbdc5a..1ed5d5d16 100644 --- a/src/util.c +++ b/src/util.c @@ -95,14 +95,16 @@ int git_libgit2_opts(int key, ...) error = git_futils_dirs_set(error, va_arg(ap, const char *)); break; - case GIT_OPT_SET_CACHE_LIMIT: { + case GIT_OPT_SET_CACHE_LIMIT: + { git_otype type = (git_otype)va_arg(ap, int); - git_cache__max_object_size[type] = va_arg(ap, size_t); + size_t size = va_arg(ap, size_t); + error = git_cache_set_max_object_size(type, size); break; } case GIT_OPT_ENABLE_CACHING: - git_cache__enabled = va_arg(ap, int); + git_cache__enabled = (va_arg(ap, int) != 0); break; } From 24c70804e87523df99f4740ed2829976ec1a9c1b Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 12 Apr 2013 12:59:38 -0700 Subject: [PATCH 13/25] Add mutex around mapping and unmapping pack files When I was writing threading tests for the new cache, the main error I kept running into was a pack file having it's content unmapped underneath the running thread. This adds a lock around the routines that map and unmap the pack data so that threads can effectively reload the data when they need it. This also required reworking the error handling paths in a couple places in the code which I tried to make consistent. --- src/pack.c | 67 +++++++++++++++++++++++++++++++++++------------------- src/pack.h | 1 + 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/pack.c b/src/pack.c index 75ac98186..1bffb4778 100644 --- a/src/pack.c +++ b/src/pack.c @@ -296,24 +296,34 @@ static int pack_index_check(const char *path, struct git_pack_file *p) static int pack_index_open(struct git_pack_file *p) { char *idx_name; - int error; - size_t name_len, offset; + int error = 0; + size_t name_len, base_len; + + if ((error = git_mutex_lock(&p->lock)) < 0) + return error; if (p->index_map.data) - return 0; + goto done; - idx_name = git__strdup(p->pack_name); - GITERR_CHECK_ALLOC(idx_name); + name_len = strlen(p->pack_name); + assert(name_len > strlen(".pack")); /* checked by git_pack_file alloc */ - name_len = strlen(idx_name); - offset = name_len - strlen(".pack"); - assert(offset < name_len); /* make sure no underflow */ + if ((idx_name = git__malloc(name_len)) == NULL) { + error = -1; + goto done; + } - strncpy(idx_name + offset, ".idx", name_len - offset); + base_len = name_len - strlen(".pack"); + memcpy(idx_name, p->pack_name, base_len); + memcpy(idx_name + base_len, ".idx", sizeof(".idx")); error = pack_index_check(idx_name, p); + git__free(idx_name); +done: + git_mutex_unlock(&p->lock); + return error; } @@ -389,7 +399,7 @@ int git_packfile_unpack_header( * the maximum deflated object size is 2^137, which is just * insane, so we know won't exceed what we have been given. */ -// base = pack_window_open(p, w_curs, *curpos, &left); +/* base = pack_window_open(p, w_curs, *curpos, &left); */ base = git_mwindow_open(mwf, w_curs, *curpos, 20, &left); if (base == NULL) return GIT_EBUFS; @@ -789,15 +799,23 @@ git_off_t get_delta_base( static struct git_pack_file *packfile_alloc(size_t extra) { struct git_pack_file *p = git__calloc(1, sizeof(*p) + extra); - if (p != NULL) - p->mwf.fd = -1; + if (!p) + return NULL; + + p->mwf.fd = -1; + git_mutex_init(&p->lock); + return p; } void git_packfile_free(struct git_pack_file *p) { - assert(p); + if (!p) + return; + + if (git_mutex_lock(&p->lock) < 0) + return; cache_free(&p->bases); @@ -810,6 +828,10 @@ void git_packfile_free(struct git_pack_file *p) pack_index_free(p); git__free(p->bad_object_sha1); + + git_mutex_unlock(&p->lock); + + git_mutex_free(&p->lock); git__free(p); } @@ -820,8 +842,6 @@ static int packfile_open(struct git_pack_file *p) git_oid sha1; unsigned char *idx_sha1; - assert(p->index_map.data); - if (!p->index_map.data && pack_index_open(p) < 0) return git_odb__error_notfound("failed to open packfile", NULL); @@ -888,7 +908,10 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path) size_t path_len; *pack_out = NULL; - path_len = strlen(path); + + if (!path || (path_len = strlen(path)) <= strlen(".idx")) + return git_odb__error_notfound("invalid packfile path", NULL); + p = packfile_alloc(path_len + 2); GITERR_CHECK_ALLOC(p); @@ -897,18 +920,13 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path) * the index looks sane. */ path_len -= strlen(".idx"); - if (path_len < 1) { - git__free(p); - return git_odb__error_notfound("invalid packfile path", NULL); - } - memcpy(p->pack_name, path, path_len); - strcpy(p->pack_name + path_len, ".keep"); + memcpy(p->pack_name + path_len, ".keep", sizeof(".keep")); if (git_path_exists(p->pack_name) == true) p->pack_keep = 1; - strcpy(p->pack_name + path_len, ".pack"); + memcpy(p->pack_name + path_len, ".pack", sizeof(".pack")); if (p_stat(p->pack_name, &st) < 0 || !S_ISREG(st.st_mode)) { git__free(p); return git_odb__error_notfound("packfile not found", NULL); @@ -1039,7 +1057,6 @@ static int pack_entry_find_offset( if ((error = pack_index_open(p)) < 0) return error; - assert(p->index_map.data); index = p->index_map.data; @@ -1099,6 +1116,7 @@ static int pack_entry_find_offset( return git_odb__error_notfound("failed to find offset for pack entry", short_oid); if (found > 1) return git_odb__error_ambiguous("found multiple offsets for pack entry"); + *offset_out = nth_packed_object_offset(p, pos); git_oid_fromraw(found_oid, current); @@ -1110,6 +1128,7 @@ static int pack_entry_find_offset( printf("found lo=%d %s\n", lo, hex_sha1); } #endif + return 0; } diff --git a/src/pack.h b/src/pack.h index 8d7e33dfe..b734ac163 100644 --- a/src/pack.h +++ b/src/pack.h @@ -79,6 +79,7 @@ typedef struct { struct git_pack_file { git_mwindow_file mwf; git_map index_map; + git_mutex lock; uint32_t num_objects; uint32_t num_bad_objects; From 917f60c50bce09f789aeb927b45ba3bca5a23877 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 12 Apr 2013 13:04:08 -0700 Subject: [PATCH 14/25] Add tests for oidmap and new cache with threading This adds some basic tests for the oidmap just to make sure that collisions, etc. are dealt with correctly. This also adds some tests for the new caching that check if items are inserted (or not inserted) properly into the cache, and that the cache can hold up in a multithreaded environment without error. --- tests-clar/core/oidmap.c | 110 ++++++++++++++++++ tests-clar/object/cache.c | 232 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 342 insertions(+) create mode 100644 tests-clar/core/oidmap.c create mode 100644 tests-clar/object/cache.c diff --git a/tests-clar/core/oidmap.c b/tests-clar/core/oidmap.c new file mode 100644 index 000000000..ec4b5e775 --- /dev/null +++ b/tests-clar/core/oidmap.c @@ -0,0 +1,110 @@ +#include "clar_libgit2.h" +#include "oidmap.h" + +GIT__USE_OIDMAP; + +typedef struct { + git_oid oid; + size_t extra; +} oidmap_item; + +#define NITEMS 0x0fff + +void test_core_oidmap__basic(void) +{ + git_oidmap *map; + oidmap_item items[NITEMS]; + uint32_t i, j; + + for (i = 0; i < NITEMS; ++i) { + items[i].extra = i; + for (j = 0; j < GIT_OID_RAWSZ / 4; ++j) { + items[i].oid.id[j * 4 ] = (unsigned char)i; + items[i].oid.id[j * 4 + 1] = (unsigned char)(i >> 8); + items[i].oid.id[j * 4 + 2] = (unsigned char)(i >> 16); + items[i].oid.id[j * 4 + 3] = (unsigned char)(i >> 24); + } + } + + map = git_oidmap_alloc(); + cl_assert(map != NULL); + + for (i = 0; i < NITEMS; ++i) { + khiter_t pos; + int ret; + + pos = kh_get(oid, map, &items[i].oid); + cl_assert(pos == kh_end(map)); + + pos = kh_put(oid, map, &items[i].oid, &ret); + cl_assert(ret != 0); + + kh_val(map, pos) = &items[i]; + } + + + for (i = 0; i < NITEMS; ++i) { + khiter_t pos; + + pos = kh_get(oid, map, &items[i].oid); + cl_assert(pos != kh_end(map)); + + cl_assert_equal_p(kh_val(map, pos), &items[i]); + } + + git_oidmap_free(map); +} + +void test_core_oidmap__hash_collision(void) +{ + git_oidmap *map; + oidmap_item items[NITEMS]; + uint32_t i, j; + + for (i = 0; i < NITEMS; ++i) { + uint32_t segment = i / 8; + int modi = i - (segment * 8); + + items[i].extra = i; + + for (j = 0; j < GIT_OID_RAWSZ / 4; ++j) { + items[i].oid.id[j * 4 ] = (unsigned char)modi; + items[i].oid.id[j * 4 + 1] = (unsigned char)(modi >> 8); + items[i].oid.id[j * 4 + 2] = (unsigned char)(modi >> 16); + items[i].oid.id[j * 4 + 3] = (unsigned char)(modi >> 24); + } + + items[i].oid.id[ 8] = (unsigned char)i; + items[i].oid.id[ 9] = (unsigned char)(i >> 8); + items[i].oid.id[10] = (unsigned char)(i >> 16); + items[i].oid.id[11] = (unsigned char)(i >> 24); + } + + map = git_oidmap_alloc(); + cl_assert(map != NULL); + + for (i = 0; i < NITEMS; ++i) { + khiter_t pos; + int ret; + + pos = kh_get(oid, map, &items[i].oid); + cl_assert(pos == kh_end(map)); + + pos = kh_put(oid, map, &items[i].oid, &ret); + cl_assert(ret != 0); + + kh_val(map, pos) = &items[i]; + } + + + for (i = 0; i < NITEMS; ++i) { + khiter_t pos; + + pos = kh_get(oid, map, &items[i].oid); + cl_assert(pos != kh_end(map)); + + cl_assert_equal_p(kh_val(map, pos), &items[i]); + } + + git_oidmap_free(map); +} diff --git a/tests-clar/object/cache.c b/tests-clar/object/cache.c new file mode 100644 index 000000000..8121247c9 --- /dev/null +++ b/tests-clar/object/cache.c @@ -0,0 +1,232 @@ +#include "clar_libgit2.h" +#include "repository.h" + +static git_repository *g_repo; + +void test_object_cache__initialize(void) +{ + cl_git_pass(git_repository_open(&g_repo, cl_fixture("testrepo.git"))); +} + +void test_object_cache__cleanup(void) +{ + git_repository_free(g_repo); + g_repo = NULL; + + git_libgit2_opts(GIT_OPT_SET_CACHE_LIMIT, (int)GIT_OBJ_BLOB, (size_t)0); +} + +static struct { + git_otype type; + const char *sha; +} g_data[] = { + /* HEAD */ + { GIT_OBJ_BLOB, "a8233120f6ad708f843d861ce2b7228ec4e3dec6" }, /* README */ + { GIT_OBJ_BLOB, "3697d64be941a53d4ae8f6a271e4e3fa56b022cc" }, /* branch_file.txt */ + { GIT_OBJ_BLOB, "a71586c1dfe8a71c6cbf6c129f404c5642ff31bd" }, /* new.txt */ + + /* refs/heads/subtrees */ + { GIT_OBJ_BLOB, "1385f264afb75a56a5bec74243be9b367ba4ca08" }, /* README */ + { GIT_OBJ_TREE, "f1425cef211cc08caa31e7b545ffb232acb098c3" }, /* ab */ + { GIT_OBJ_BLOB, "d6c93164c249c8000205dd4ec5cbca1b516d487f" }, /* ab/4.txt */ + { GIT_OBJ_TREE, "9a03079b8a8ee85a0bee58bf9be3da8b62414ed4" }, /* ab/c */ + { GIT_OBJ_BLOB, "270b8ea76056d5cad83af921837702d3e3c2924d" }, /* ab/c/3.txt */ + { GIT_OBJ_TREE, "b6361fc6a97178d8fc8639fdeed71c775ab52593" }, /* ab/de */ + { GIT_OBJ_BLOB, "e7b4ad382349ff96dd8199000580b9b1e2042eb0" }, /* ab/de/2.txt */ + { GIT_OBJ_TREE, "3259a6bd5b57fb9c1281bb7ed3167b50f224cb54" }, /* ab/de/fgh */ + { GIT_OBJ_BLOB, "1f67fc4386b2d171e0d21be1c447e12660561f9b" }, /* ab/de/fgh/1.txt */ + { GIT_OBJ_BLOB, "45b983be36b73c0788dc9cbcb76cbb80fc7bb057" }, /* branch_file.txt */ + { GIT_OBJ_BLOB, "fa49b077972391ad58037050f2a75f74e3671e92" }, /* new.txt */ + + /* refs/heads/chomped */ + { GIT_OBJ_BLOB, "0266163a49e280c4f5ed1e08facd36a2bd716bcf" }, /* readme.txt */ + + { 0, NULL }, + { 0, NULL } +}; + +void test_object_cache__cache_everything(void) +{ + int i, start; + git_oid oid; + git_odb_object *odb_obj; + git_object *obj; + git_odb *odb; + + git_libgit2_opts( + GIT_OPT_SET_CACHE_LIMIT, (int)GIT_OBJ_BLOB, (size_t)32767); + + cl_git_pass(git_repository_odb(&odb, g_repo)); + + start = (int)git_cache_size(&g_repo->objects); + + for (i = 0; g_data[i].sha != NULL; ++i) { + int count = (int)git_cache_size(&g_repo->objects); + + cl_git_pass(git_oid_fromstr(&oid, g_data[i].sha)); + + /* alternate between loading raw and parsed objects */ + if ((i & 1) == 0) { + cl_git_pass(git_odb_read(&odb_obj, odb, &oid)); + cl_assert(g_data[i].type == git_odb_object_type(odb_obj)); + git_odb_object_free(odb_obj); + } else { + cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY)); + cl_assert(g_data[i].type == git_object_type(obj)); + git_object_free(obj); + } + + cl_assert_equal_i(count + 1, (int)git_cache_size(&g_repo->objects)); + } + + cl_assert_equal_i(i, git_cache_size(&g_repo->objects) - start); + + git_odb_free(odb); + + for (i = 0; g_data[i].sha != NULL; ++i) { + int count = (int)git_cache_size(&g_repo->objects); + + cl_git_pass(git_oid_fromstr(&oid, g_data[i].sha)); + cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY)); + cl_assert(g_data[i].type == git_object_type(obj)); + git_object_free(obj); + + cl_assert_equal_i(count, (int)git_cache_size(&g_repo->objects)); + } +} + +void test_object_cache__cache_no_blobs(void) +{ + int i, start, nonblobs = 0; + git_oid oid; + git_odb_object *odb_obj; + git_object *obj; + git_odb *odb; + + git_libgit2_opts(GIT_OPT_SET_CACHE_LIMIT, (int)GIT_OBJ_BLOB, (size_t)0); + + cl_git_pass(git_repository_odb(&odb, g_repo)); + + start = (int)git_cache_size(&g_repo->objects); + + for (i = 0; g_data[i].sha != NULL; ++i) { + int count = (int)git_cache_size(&g_repo->objects); + + cl_git_pass(git_oid_fromstr(&oid, g_data[i].sha)); + + /* alternate between loading raw and parsed objects */ + if ((i & 1) == 0) { + cl_git_pass(git_odb_read(&odb_obj, odb, &oid)); + cl_assert(g_data[i].type == git_odb_object_type(odb_obj)); + git_odb_object_free(odb_obj); + } else { + cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY)); + cl_assert(g_data[i].type == git_object_type(obj)); + git_object_free(obj); + } + + if (g_data[i].type == GIT_OBJ_BLOB) + cl_assert_equal_i(count, (int)git_cache_size(&g_repo->objects)); + else { + cl_assert_equal_i(count + 1, (int)git_cache_size(&g_repo->objects)); + nonblobs++; + } + } + + cl_assert_equal_i(nonblobs, git_cache_size(&g_repo->objects) - start); + + git_odb_free(odb); +} + +static void *cache_parsed(void *arg) +{ + int i; + git_oid oid; + git_object *obj; + + for (i = ((int *)arg)[1]; g_data[i].sha != NULL; i += 2) { + cl_git_pass(git_oid_fromstr(&oid, g_data[i].sha)); + cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY)); + cl_assert(g_data[i].type == git_object_type(obj)); + git_object_free(obj); + } + + for (i = 0; i < ((int *)arg)[1]; i += 2) { + cl_git_pass(git_oid_fromstr(&oid, g_data[i].sha)); + cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY)); + cl_assert(g_data[i].type == git_object_type(obj)); + git_object_free(obj); + } + + return arg; +} + +static void *cache_raw(void *arg) +{ + int i; + git_oid oid; + git_odb *odb; + git_odb_object *odb_obj; + + cl_git_pass(git_repository_odb(&odb, g_repo)); + + for (i = ((int *)arg)[1]; g_data[i].sha != NULL; i += 2) { + cl_git_pass(git_oid_fromstr(&oid, g_data[i].sha)); + cl_git_pass(git_odb_read(&odb_obj, odb, &oid)); + cl_assert(g_data[i].type == git_odb_object_type(odb_obj)); + git_odb_object_free(odb_obj); + } + + for (i = 0; i < ((int *)arg)[1]; i += 2) { + cl_git_pass(git_oid_fromstr(&oid, g_data[i].sha)); + cl_git_pass(git_odb_read(&odb_obj, odb, &oid)); + cl_assert(g_data[i].type == git_odb_object_type(odb_obj)); + git_odb_object_free(odb_obj); + } + + git_odb_free(odb); + + return arg; +} + +#define REPEAT 50 +#define THREADCOUNT 20 + +void test_object_cache__threadmania(void) +{ + int try, th, max_i; + git_thread t[THREADCOUNT]; + void *data; + void *(*fn)(void *); + + for (max_i = 0; g_data[max_i].sha != NULL; ++max_i) + /* count up */; + + for (try = 0; try < REPEAT; ++try) { + + for (th = 0; th < THREADCOUNT; ++th) { + data = git__malloc(2 * sizeof(int)); + + ((int *)data)[0] = th; + ((int *)data)[1] = th % max_i; + + fn = (th & 1) ? cache_parsed : cache_raw; + +#ifdef GIT_THREADS + git_thread_create(&t[th], NULL, fn, data); +#else + fn(data); + git__free(data); +#endif + } + +#ifdef GIT_THREADS + for (th = 0; th < THREADCOUNT; ++th) { + git_thread_join(t[th], &data); + cl_assert_equal_i(th, ((int *)data)[0]); + git__free(data); + } +#endif + + } +} From 786062639f05e361da977f3f1f6286141fa12fca Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 15 Apr 2013 00:05:44 -0700 Subject: [PATCH 15/25] Add callback to git_objects_table This adds create and free callback to the git_objects_table so that more of the creation and destruction of objects can be table driven instead of using switch statements. This also makes the semantics of certain object creation functions consistent so that we can make better use of function pointers. This also fixes a theoretical error case where an object allocation fails and we end up storing NULL into the cache. --- src/blob.c | 8 +-- src/blob.h | 4 +- src/cache.c | 14 ++-- src/cache.h | 12 ++-- src/commit.c | 16 ++--- src/commit.h | 5 +- src/object.c | 145 ++++++++++++++------------------------ src/odb.c | 31 +++++--- src/odb.h | 5 +- src/oidmap.h | 4 +- src/tag.c | 19 ++--- src/tag.h | 5 +- src/tree.c | 27 +++---- src/tree.h | 4 +- tests-clar/commit/parse.c | 13 ++-- 15 files changed, 128 insertions(+), 184 deletions(-) diff --git a/src/blob.c b/src/blob.c index 732b0f3de..501c13d1a 100644 --- a/src/blob.c +++ b/src/blob.c @@ -35,17 +35,17 @@ int git_blob__getbuf(git_buf *buffer, git_blob *blob) git_odb_object_size(blob->odb_object)); } -void git_blob__free(git_blob *blob) +void git_blob__free(void *blob) { - git_odb_object_free(blob->odb_object); + git_odb_object_free(((git_blob *)blob)->odb_object); git__free(blob); } -int git_blob__parse(git_blob *blob, git_odb_object *odb_obj) +int git_blob__from_odb_object(void *blob, git_odb_object *odb_obj) { assert(blob); git_cached_obj_incref((git_cached_obj *)odb_obj); - blob->odb_object = odb_obj; + ((git_blob *)blob)->odb_object = odb_obj; return 0; } diff --git a/src/blob.h b/src/blob.h index 524734b1f..4873505fd 100644 --- a/src/blob.h +++ b/src/blob.h @@ -17,8 +17,8 @@ struct git_blob { git_odb_object *odb_object; }; -void git_blob__free(git_blob *blob); -int git_blob__parse(git_blob *blob, git_odb_object *obj); +void git_blob__free(void *blob); +int git_blob__from_odb_object(void *blob, git_odb_object *obj); int git_blob__getbuf(git_buf *buffer, git_blob *blob); #endif diff --git a/src/cache.c b/src/cache.c index 263f736fa..c51be895e 100644 --- a/src/cache.c +++ b/src/cache.c @@ -70,12 +70,6 @@ int git_cache_init(git_cache *cache) return 0; } -void git_cache_free(git_cache *cache) -{ - git_oidmap_free(cache->map); - git_mutex_free(&cache->lock); -} - void git_cache_clear(git_cache *cache) { git_cached_obj *evict = NULL; @@ -93,6 +87,14 @@ void git_cache_clear(git_cache *cache) git_mutex_unlock(&cache->lock); } +void git_cache_free(git_cache *cache) +{ + git_cache_clear(cache); + + git_oidmap_free(cache->map); + git_mutex_free(&cache->lock); +} + /* Call with lock, yo */ static void cache_evict_entries(git_cache *cache, size_t evict_count) { diff --git a/src/cache.h b/src/cache.h index 13b630e89..e95d521fe 100644 --- a/src/cache.h +++ b/src/cache.h @@ -21,17 +21,17 @@ enum { }; typedef struct { - git_oid oid; - int16_t type; - uint16_t flags; - size_t size; + git_oid oid; + int16_t type; /* git_otype value */ + uint16_t flags; /* GIT_CACHE_STORE value */ + size_t size; git_atomic refcount; } git_cached_obj; typedef struct { git_oidmap *map; - git_mutex lock; - size_t used_memory; + git_mutex lock; + size_t used_memory; } git_cache; extern bool git_cache__enabled; diff --git a/src/commit.c b/src/commit.c index 2cee44cd2..3eca5b341 100644 --- a/src/commit.c +++ b/src/commit.c @@ -31,8 +31,10 @@ static void clear_parents(git_commit *commit) git_vector_clear(&commit->parent_ids); } -void git_commit__free(git_commit *commit) +void git_commit__free(void *_commit) { + git_commit *commit = _commit; + clear_parents(commit); git_vector_free(&commit->parent_ids); @@ -166,10 +168,9 @@ int git_commit_create( return retval; } -int git_commit__parse_buffer(git_commit *commit, const void *data, size_t len) +int git_commit__parse(void *_commit, const char *buffer, const char *buffer_end) { - const char *buffer = data; - const char *buffer_end = (const char *)data + len; + git_commit *commit = _commit; git_oid parent_id; if (git_vector_init(&commit->parent_ids, 4, NULL) < 0) @@ -241,13 +242,6 @@ bad_buffer: return -1; } -int git_commit__parse(git_commit *commit, git_odb_object *obj) -{ - assert(commit); - return git_commit__parse_buffer( - commit, git_odb_object_data(obj), git_odb_object_size(obj)); -} - #define GIT_COMMIT_GETTER(_rvalue, _name, _return) \ _rvalue git_commit_##_name(const git_commit *commit) \ {\ diff --git a/src/commit.h b/src/commit.h index 1ab164c0b..0c2c3ab5d 100644 --- a/src/commit.h +++ b/src/commit.h @@ -27,8 +27,7 @@ struct git_commit { char *message; }; -void git_commit__free(git_commit *c); -int git_commit__parse(git_commit *commit, git_odb_object *obj); +void git_commit__free(void *commit); +int git_commit__parse(void *commit, const char *buf, const char *bufend); -int git_commit__parse_buffer(git_commit *commit, const void *data, size_t len); #endif diff --git a/src/object.c b/src/object.c index 80b765ef9..3698808c3 100644 --- a/src/object.c +++ b/src/object.c @@ -18,63 +18,39 @@ static const int OBJECT_BASE_SIZE = 4096; -static struct { +typedef struct { const char *str; /* type name string */ - int loose; /* valid loose object type flag */ size_t size; /* size in bytes of the object structure */ -} git_objects_table[] = { + + int (*from_odb)(void *self, git_odb_object *obj); + int (*parse)(void *self, const char *buf, const char *buf_end); + void (*free)(void *self); +} git_object_def; + +static git_object_def git_objects_table[] = { /* 0 = GIT_OBJ__EXT1 */ - { "", 0, 0}, + { "", 0, NULL, NULL, NULL }, /* 1 = GIT_OBJ_COMMIT */ - { "commit", 1, sizeof(struct git_commit)}, + { "commit", sizeof(git_commit), NULL, git_commit__parse, git_commit__free }, /* 2 = GIT_OBJ_TREE */ - { "tree", 1, sizeof(struct git_tree) }, + { "tree", sizeof(git_tree), NULL, git_tree__parse, git_tree__free }, /* 3 = GIT_OBJ_BLOB */ - { "blob", 1, sizeof(struct git_blob) }, + { "blob", sizeof(git_blob), git_blob__from_odb_object, NULL, git_blob__free }, /* 4 = GIT_OBJ_TAG */ - { "tag", 1, sizeof(struct git_tag) }, + { "tag", sizeof(git_tag), NULL, git_tag__parse, git_tag__free }, /* 5 = GIT_OBJ__EXT2 */ - { "", 0, 0 }, - + { "", 0, NULL, NULL, NULL }, /* 6 = GIT_OBJ_OFS_DELTA */ - { "OFS_DELTA", 0, 0 }, - + { "OFS_DELTA", 0, NULL, NULL, NULL }, /* 7 = GIT_OBJ_REF_DELTA */ - { "REF_DELTA", 0, 0 } + { "REF_DELTA", 0, NULL, NULL, NULL }, }; -static int create_object(git_object **object_out, git_otype type) -{ - git_object *object = NULL; - - assert(object_out); - - *object_out = NULL; - - switch (type) { - case GIT_OBJ_COMMIT: - case GIT_OBJ_TAG: - case GIT_OBJ_BLOB: - case GIT_OBJ_TREE: - object = git__malloc(git_object__size(type)); - GITERR_CHECK_ALLOC(object); - memset(object, 0x0, git_object__size(type)); - break; - - default: - giterr_set(GITERR_INVALID, "The given type is invalid"); - return -1; - } - - *object_out = object; - return 0; -} - int git_object__from_odb_object( git_object **object_out, git_repository *repo, @@ -82,46 +58,47 @@ int git_object__from_odb_object( git_otype type) { int error; + size_t object_size; + git_object_def *def; git_object *object = NULL; + assert(object_out); + *object_out = NULL; + + /* Validate type match */ if (type != GIT_OBJ_ANY && type != odb_obj->cached.type) { giterr_set(GITERR_INVALID, "The requested type does not match the type in the ODB"); return GIT_ENOTFOUND; } - if ((error = create_object(&object, odb_obj->cached.type)) < 0) - return error; + if ((object_size = git_object__size(odb_obj->cached.type)) == 0) { + giterr_set(GITERR_INVALID, "The requested type is invalid"); + return GIT_ENOTFOUND; + } + + /* Allocate and initialize base object */ + object = git__calloc(1, object_size); + GITERR_CHECK_ALLOC(object); - /* Initialize parent object */ git_oid_cpy(&object->cached.oid, &odb_obj->cached.oid); - object->cached.size = odb_obj->cached.size; object->cached.type = odb_obj->cached.type; + object->cached.size = odb_obj->cached.size; object->repo = repo; - switch (object->cached.type) { - case GIT_OBJ_COMMIT: - error = git_commit__parse((git_commit *)object, odb_obj); - break; + /* Parse raw object data */ + def = &git_objects_table[odb_obj->cached.type]; + assert(def->free && (def->from_odb || def->parse)); - case GIT_OBJ_TREE: - error = git_tree__parse((git_tree *)object, odb_obj); - break; - - case GIT_OBJ_TAG: - error = git_tag__parse((git_tag *)object, odb_obj); - break; - - case GIT_OBJ_BLOB: - error = git_blob__parse((git_blob *)object, odb_obj); - break; - - default: - break; + if (def->from_odb) { + error = def->from_odb(object, odb_obj); + } else { + const char *data = (const char *)git_odb_object_data(odb_obj); + error = def->parse(object, data, data + git_odb_object_size(odb_obj)); } if (error < 0) { - git_object__free(object); + def->free(object); return error; } @@ -129,6 +106,17 @@ int git_object__from_odb_object( return 0; } +void git_object__free(void *obj) +{ + git_otype type = ((git_object *)obj)->cached.type; + + if (type < 0 || ((size_t)type) >= ARRAY_SIZE(git_objects_table) || + !git_objects_table[type].free) + git__free(obj); + else + git_objects_table[type].free(obj); +} + int git_object_lookup_prefix( git_object **object_out, git_repository *repo, @@ -222,35 +210,6 @@ int git_object_lookup(git_object **object_out, git_repository *repo, const git_o return git_object_lookup_prefix(object_out, repo, id, GIT_OID_HEXSZ, type); } -void git_object__free(void *_obj) -{ - git_object *object = (git_object *)_obj; - - assert(object); - - switch (object->cached.type) { - case GIT_OBJ_COMMIT: - git_commit__free((git_commit *)object); - break; - - case GIT_OBJ_TREE: - git_tree__free((git_tree *)object); - break; - - case GIT_OBJ_TAG: - git_tag__free((git_tag *)object); - break; - - case GIT_OBJ_BLOB: - git_blob__free((git_blob *)object); - break; - - default: - git__free(object); - break; - } -} - void git_object_free(git_object *object) { if (object == NULL) @@ -304,7 +263,7 @@ int git_object_typeisloose(git_otype type) if (type < 0 || ((size_t) type) >= ARRAY_SIZE(git_objects_table)) return 0; - return git_objects_table[type].loose; + return (git_objects_table[type].size > 0) ? 1 : 0; } size_t git_object__size(git_otype type) diff --git a/src/odb.c b/src/odb.c index 16a842aa8..53630dddc 100644 --- a/src/odb.c +++ b/src/odb.c @@ -82,23 +82,24 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj) } -static git_odb_object *new_odb_object(const git_oid *oid, git_rawobj *source) +static git_odb_object *odb_object__alloc(const git_oid *oid, git_rawobj *source) { - git_odb_object *object = git__malloc(sizeof(git_odb_object)); - memset(object, 0x0, sizeof(git_odb_object)); + git_odb_object *object = git__calloc(1, sizeof(git_odb_object)); - git_oid_cpy(&object->cached.oid, oid); - object->cached.size = source->len; - object->cached.type = source->type; - object->buffer = source->data; + if (object != NULL) { + git_oid_cpy(&object->cached.oid, oid); + object->cached.type = source->type; + object->cached.size = source->len; + object->buffer = source->data; + } return object; } -void git_odb_object__free(git_odb_object *object) +void git_odb_object__free(void *object) { if (object != NULL) { - git__free(object->buffer); + git__free(((git_odb_object *)object)->buffer); git__free(object); } } @@ -679,6 +680,7 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) int error; bool refreshed = false; git_rawobj raw; + git_odb_object *object; assert(out && db && id); @@ -713,7 +715,10 @@ attempt_lookup: if (error && error != GIT_PASSTHROUGH) return error; - *out = git_cache_store_raw(odb_cache(db), new_odb_object(id, &raw)); + if ((object = odb_object__alloc(id, &raw)) == NULL) + return -1; + + *out = git_cache_store_raw(odb_cache(db), object); return 0; } @@ -726,6 +731,7 @@ int git_odb_read_prefix( git_rawobj raw; void *data = NULL; bool found = false, refreshed = false; + git_odb_object *object; assert(out && db); @@ -777,7 +783,10 @@ attempt_lookup: if (!found) return git_odb__error_notfound("no match for prefix", short_id); - *out = git_cache_store_raw(odb_cache(db), new_odb_object(&found_full_oid, &raw)); + if ((object = odb_object__alloc(&found_full_oid, &raw)) == NULL) + return -1; + + *out = git_cache_store_raw(odb_cache(db), object); return 0; } diff --git a/src/odb.h b/src/odb.h index 22c6e1668..0d9f9e2ea 100644 --- a/src/odb.h +++ b/src/odb.h @@ -39,8 +39,6 @@ struct git_odb { git_cache own_cache; }; -void git_odb_object__free(git_odb_object *object); - /* * Hash a git_rawobj internally. * The `git_rawobj` is supposed to be previously initialized @@ -98,4 +96,7 @@ int git_odb__read_header_or_object( git_odb_object **out, size_t *len_p, git_otype *type_p, git_odb *db, const git_oid *id); +/* fully free the object; internal method, DO NOT EXPORT */ +void git_odb_object__free(void *object); + #endif diff --git a/src/oidmap.h b/src/oidmap.h index dfa951af3..a29c7cd35 100644 --- a/src/oidmap.h +++ b/src/oidmap.h @@ -19,7 +19,7 @@ __KHASH_TYPE(oid, const git_oid *, void *); typedef khash_t(oid) git_oidmap; -GIT_INLINE(khint_t) hash_git_oid(const git_oid *oid) +GIT_INLINE(khint_t) git_oidmap_hash(const git_oid *oid) { khint_t h; memcpy(&h, oid, sizeof(khint_t)); @@ -27,7 +27,7 @@ GIT_INLINE(khint_t) hash_git_oid(const git_oid *oid) } #define GIT__USE_OIDMAP \ - __KHASH_IMPL(oid, static kh_inline, const git_oid *, void *, 1, hash_git_oid, git_oid_equal) + __KHASH_IMPL(oid, static kh_inline, const git_oid *, void *, 1, git_oidmap_hash, git_oid_equal) #define git_oidmap_alloc() kh_init(oid) #define git_oidmap_free(h) kh_destroy(oid,h), h = NULL diff --git a/src/tag.c b/src/tag.c index b76895d0c..7dadc7e60 100644 --- a/src/tag.c +++ b/src/tag.c @@ -15,8 +15,9 @@ #include "git2/signature.h" #include "git2/odb_backend.h" -void git_tag__free(git_tag *tag) +void git_tag__free(void *_tag) { + git_tag *tag = _tag; git_signature_free(tag->tagger); git__free(tag->message); git__free(tag->tag_name); @@ -69,18 +70,17 @@ static int tag_error(const char *str) return -1; } -int git_tag__parse_buffer(git_tag *tag, const char *buffer, size_t length) +int git_tag__parse(void *_tag, const char *buffer, const char *buffer_end) { static const char *tag_types[] = { NULL, "commit\n", "tree\n", "blob\n", "tag\n" }; + git_tag *tag = _tag; unsigned int i; size_t text_len; char *search; - const char *buffer_end = buffer + length; - if (git_oid__parse(&tag->target, &buffer, buffer_end, "object ") < 0) return tag_error("Object field invalid"); @@ -317,7 +317,7 @@ int git_tag_create_frombuffer(git_oid *oid, git_repository *repo, const char *bu return -1; /* validate the buffer */ - if (git_tag__parse_buffer(&tag, buffer, strlen(buffer)) < 0) + if (git_tag__parse(&tag, buffer, buffer + strlen(buffer)) < 0) return -1; /* validate the target */ @@ -390,15 +390,8 @@ int git_tag_delete(git_repository *repo, const char *tag_name) if ((error = git_reference_delete(tag_ref)) == 0) git_reference_free(tag_ref); - - return error; -} -int git_tag__parse(git_tag *tag, git_odb_object *obj) -{ - assert(tag); - return git_tag__parse_buffer( - tag, git_odb_object_data(obj), git_odb_object_size(obj)); + return error; } typedef struct { diff --git a/src/tag.h b/src/tag.h index c8e421ee6..fb01a6f75 100644 --- a/src/tag.h +++ b/src/tag.h @@ -22,8 +22,7 @@ struct git_tag { char *message; }; -void git_tag__free(git_tag *tag); -int git_tag__parse(git_tag *tag, git_odb_object *obj); -int git_tag__parse_buffer(git_tag *tag, const char *data, size_t len); +void git_tag__free(void *tag); +int git_tag__parse(void *tag, const char *buf, const char *buf_end); #endif diff --git a/src/tree.c b/src/tree.c index cc43b920c..e66fa2370 100644 --- a/src/tree.c +++ b/src/tree.c @@ -219,15 +219,16 @@ git_tree_entry *git_tree_entry_dup(const git_tree_entry *entry) return copy; } -void git_tree__free(git_tree *tree) +void git_tree__free(void *tree) { + git_vector *entries = &((git_tree *)tree)->entries; size_t i; git_tree_entry *e; - git_vector_foreach(&tree->entries, i, e) + git_vector_foreach(entries, i, e) git_tree_entry_free(e); - git_vector_free(&tree->entries); + git_vector_free(entries); git__free(tree); } @@ -371,10 +372,11 @@ static int tree_error(const char *str, const char *path) return -1; } -static int tree_parse_buffer( - git_tree *tree, const char *buffer, const char *buffer_end) +int git_tree__parse(void *tree, const char *buffer, const char *buffer_end) { - if (git_vector_init(&tree->entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) + git_vector *tree_entries = &((git_tree *)tree)->entries; + + if (git_vector_init(tree_entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) return -1; while (buffer < buffer_end) { @@ -397,7 +399,7 @@ static int tree_parse_buffer( entry = alloc_entry(buffer); GITERR_CHECK_ALLOC(entry); - if (git_vector_insert(&tree->entries, entry) < 0) { + if (git_vector_insert(tree_entries, entry) < 0) { git__free(entry); return -1; } @@ -417,17 +419,6 @@ static int tree_parse_buffer( return 0; } -int git_tree__parse(git_tree *tree, git_odb_object *obj) -{ - const char *buf; - - assert(tree && obj); - - buf = (const char *)git_odb_object_data(obj); - - return tree_parse_buffer(tree, buf, buf + git_odb_object_size(obj)); -} - static size_t find_next_dir(const char *dirname, git_index *index, size_t start) { size_t dirlen, i, entries = git_index_entrycount(index); diff --git a/src/tree.h b/src/tree.h index b77bfd961..cf47fb478 100644 --- a/src/tree.h +++ b/src/tree.h @@ -37,8 +37,8 @@ GIT_INLINE(bool) git_tree_entry__is_tree(const struct git_tree_entry *e) extern int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2); -void git_tree__free(git_tree *tree); -int git_tree__parse(git_tree *tree, git_odb_object *obj); +void git_tree__free(void *tree); +int git_tree__parse(void *tree, const char *buf, const char *buf_end); /** * Lookup the first position in the tree with a given prefix. diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index b99d27991..792b57626 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -269,6 +269,7 @@ void test_commit_parse__entire_commit(void) const int broken_commit_count = sizeof(failing_commit_cases) / sizeof(*failing_commit_cases); const int working_commit_count = sizeof(passing_commit_cases) / sizeof(*passing_commit_cases); int i; + const char *buf; for (i = 0; i < broken_commit_count; ++i) { git_commit *commit; @@ -276,9 +277,8 @@ void test_commit_parse__entire_commit(void) memset(commit, 0x0, sizeof(git_commit)); commit->object.repo = g_repo; - cl_git_fail(git_commit__parse_buffer( - commit, failing_commit_cases[i], strlen(failing_commit_cases[i])) - ); + buf = failing_commit_cases[i]; + cl_git_fail(git_commit__parse(commit, buf, buf + strlen(buf))); git_commit__free(commit); } @@ -290,11 +290,8 @@ void test_commit_parse__entire_commit(void) memset(commit, 0x0, sizeof(git_commit)); commit->object.repo = g_repo; - cl_git_pass(git_commit__parse_buffer( - commit, - passing_commit_cases[i], - strlen(passing_commit_cases[i])) - ); + buf = passing_commit_cases[i]; + cl_git_pass(git_commit__parse(commit, buf, buf + strlen(buf))); if (!i) cl_assert_equal_s("", git_commit_message(commit)); From 3f27127d15dfe69943d3c9ddf96d09a2300de3a9 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 16 Apr 2013 11:51:02 -0700 Subject: [PATCH 16/25] Simplify object table parse functions This unifies the object parse functions into one signature that takes an odb_object. --- src/blob.c | 2 +- src/blob.h | 2 +- src/commit.c | 4 +++- src/commit.h | 2 +- src/object.c | 37 ++++++++++++++----------------------- src/tag.c | 38 +++++++++++++++++++++++--------------- src/tag.h | 2 +- src/tree.c | 4 +++- src/tree.h | 2 +- tests-clar/commit/parse.c | 27 +++++++++++++++------------ 10 files changed, 63 insertions(+), 57 deletions(-) diff --git a/src/blob.c b/src/blob.c index 501c13d1a..a68c4cc3e 100644 --- a/src/blob.c +++ b/src/blob.c @@ -41,7 +41,7 @@ void git_blob__free(void *blob) git__free(blob); } -int git_blob__from_odb_object(void *blob, git_odb_object *odb_obj) +int git_blob__parse(void *blob, git_odb_object *odb_obj) { assert(blob); git_cached_obj_incref((git_cached_obj *)odb_obj); diff --git a/src/blob.h b/src/blob.h index 4873505fd..22e37cc3a 100644 --- a/src/blob.h +++ b/src/blob.h @@ -18,7 +18,7 @@ struct git_blob { }; void git_blob__free(void *blob); -int git_blob__from_odb_object(void *blob, git_odb_object *obj); +int git_blob__parse(void *blob, git_odb_object *obj); int git_blob__getbuf(git_buf *buffer, git_blob *blob); #endif diff --git a/src/commit.c b/src/commit.c index 3eca5b341..46c02c292 100644 --- a/src/commit.c +++ b/src/commit.c @@ -168,9 +168,11 @@ int git_commit_create( return retval; } -int git_commit__parse(void *_commit, const char *buffer, const char *buffer_end) +int git_commit__parse(void *_commit, git_odb_object *odb_obj) { git_commit *commit = _commit; + const char *buffer = git_odb_object_data(odb_obj); + const char *buffer_end = buffer + git_odb_object_size(odb_obj); git_oid parent_id; if (git_vector_init(&commit->parent_ids, 4, NULL) < 0) diff --git a/src/commit.h b/src/commit.h index 0c2c3ab5d..d0981b125 100644 --- a/src/commit.h +++ b/src/commit.h @@ -28,6 +28,6 @@ struct git_commit { }; void git_commit__free(void *commit); -int git_commit__parse(void *commit, const char *buf, const char *bufend); +int git_commit__parse(void *commit, git_odb_object *obj); #endif diff --git a/src/object.c b/src/object.c index 3698808c3..b87a07404 100644 --- a/src/object.c +++ b/src/object.c @@ -22,33 +22,32 @@ typedef struct { const char *str; /* type name string */ size_t size; /* size in bytes of the object structure */ - int (*from_odb)(void *self, git_odb_object *obj); - int (*parse)(void *self, const char *buf, const char *buf_end); + int (*parse)(void *self, git_odb_object *obj); void (*free)(void *self); } git_object_def; static git_object_def git_objects_table[] = { /* 0 = GIT_OBJ__EXT1 */ - { "", 0, NULL, NULL, NULL }, + { "", 0, NULL, NULL }, /* 1 = GIT_OBJ_COMMIT */ - { "commit", sizeof(git_commit), NULL, git_commit__parse, git_commit__free }, + { "commit", sizeof(git_commit), git_commit__parse, git_commit__free }, /* 2 = GIT_OBJ_TREE */ - { "tree", sizeof(git_tree), NULL, git_tree__parse, git_tree__free }, + { "tree", sizeof(git_tree), git_tree__parse, git_tree__free }, /* 3 = GIT_OBJ_BLOB */ - { "blob", sizeof(git_blob), git_blob__from_odb_object, NULL, git_blob__free }, + { "blob", sizeof(git_blob), git_blob__parse, git_blob__free }, /* 4 = GIT_OBJ_TAG */ - { "tag", sizeof(git_tag), NULL, git_tag__parse, git_tag__free }, + { "tag", sizeof(git_tag), git_tag__parse, git_tag__free }, /* 5 = GIT_OBJ__EXT2 */ - { "", 0, NULL, NULL, NULL }, + { "", 0, NULL, NULL }, /* 6 = GIT_OBJ_OFS_DELTA */ - { "OFS_DELTA", 0, NULL, NULL, NULL }, + { "OFS_DELTA", 0, NULL, NULL }, /* 7 = GIT_OBJ_REF_DELTA */ - { "REF_DELTA", 0, NULL, NULL, NULL }, + { "REF_DELTA", 0, NULL, NULL }, }; int git_object__from_odb_object( @@ -88,22 +87,14 @@ int git_object__from_odb_object( /* Parse raw object data */ def = &git_objects_table[odb_obj->cached.type]; - assert(def->free && (def->from_odb || def->parse)); + assert(def->free && def->parse); - if (def->from_odb) { - error = def->from_odb(object, odb_obj); - } else { - const char *data = (const char *)git_odb_object_data(odb_obj); - error = def->parse(object, data, data + git_odb_object_size(odb_obj)); - } - - if (error < 0) { + if ((error = def->parse(object, odb_obj)) < 0) def->free(object); - return error; - } + else + *object_out = git_cache_store_parsed(&repo->objects, object); - *object_out = git_cache_store_parsed(&repo->objects, object); - return 0; + return error; } void git_object__free(void *obj) diff --git a/src/tag.c b/src/tag.c index 7dadc7e60..b9a806cd1 100644 --- a/src/tag.c +++ b/src/tag.c @@ -70,13 +70,12 @@ static int tag_error(const char *str) return -1; } -int git_tag__parse(void *_tag, const char *buffer, const char *buffer_end) +static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) { static const char *tag_types[] = { NULL, "commit\n", "tree\n", "blob\n", "tag\n" }; - git_tag *tag = _tag; unsigned int i; size_t text_len; char *search; @@ -157,6 +156,15 @@ int git_tag__parse(void *_tag, const char *buffer, const char *buffer_end) return 0; } +int git_tag__parse(void *_tag, git_odb_object *odb_obj) +{ + git_tag *tag = _tag; + const char *buffer = git_odb_object_data(odb_obj); + const char *buffer_end = buffer + git_odb_object_size(odb_obj); + + return tag_parse(tag, buffer, buffer_end); +} + static int retrieve_tag_reference( git_reference **tag_reference_out, git_buf *ref_name_out, @@ -277,23 +285,23 @@ cleanup: } int git_tag_create( - git_oid *oid, - git_repository *repo, - const char *tag_name, - const git_object *target, - const git_signature *tagger, - const char *message, - int allow_ref_overwrite) + git_oid *oid, + git_repository *repo, + const char *tag_name, + const git_object *target, + const git_signature *tagger, + const char *message, + int allow_ref_overwrite) { return git_tag_create__internal(oid, repo, tag_name, target, tagger, message, allow_ref_overwrite, 1); } int git_tag_create_lightweight( - git_oid *oid, - git_repository *repo, - const char *tag_name, - const git_object *target, - int allow_ref_overwrite) + git_oid *oid, + git_repository *repo, + const char *tag_name, + const git_object *target, + int allow_ref_overwrite) { return git_tag_create__internal(oid, repo, tag_name, target, NULL, NULL, allow_ref_overwrite, 0); } @@ -317,7 +325,7 @@ int git_tag_create_frombuffer(git_oid *oid, git_repository *repo, const char *bu return -1; /* validate the buffer */ - if (git_tag__parse(&tag, buffer, buffer + strlen(buffer)) < 0) + if (tag_parse(&tag, buffer, buffer + strlen(buffer)) < 0) return -1; /* validate the target */ diff --git a/src/tag.h b/src/tag.h index fb01a6f75..d0cd393c7 100644 --- a/src/tag.h +++ b/src/tag.h @@ -23,6 +23,6 @@ struct git_tag { }; void git_tag__free(void *tag); -int git_tag__parse(void *tag, const char *buf, const char *buf_end); +int git_tag__parse(void *tag, git_odb_object *obj); #endif diff --git a/src/tree.c b/src/tree.c index e66fa2370..d6d4b77d3 100644 --- a/src/tree.c +++ b/src/tree.c @@ -372,8 +372,10 @@ static int tree_error(const char *str, const char *path) return -1; } -int git_tree__parse(void *tree, const char *buffer, const char *buffer_end) +int git_tree__parse(void *tree, git_odb_object *odb_obj) { + const char *buffer = git_odb_object_data(odb_obj); + const char *buffer_end = buffer + git_odb_object_size(odb_obj); git_vector *tree_entries = &((git_tree *)tree)->entries; if (git_vector_init(tree_entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) diff --git a/src/tree.h b/src/tree.h index cf47fb478..7cb2dd36c 100644 --- a/src/tree.h +++ b/src/tree.h @@ -38,7 +38,7 @@ GIT_INLINE(bool) git_tree_entry__is_tree(const struct git_tree_entry *e) extern int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2); void git_tree__free(void *tree); -int git_tree__parse(void *tree, const char *buf, const char *buf_end); +int git_tree__parse(void *tree, git_odb_object *obj); /** * Lookup the first position in the tree with a given prefix. diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index 792b57626..ad2c746ca 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -266,32 +266,35 @@ a simple commit which works\n", void test_commit_parse__entire_commit(void) { - const int broken_commit_count = sizeof(failing_commit_cases) / sizeof(*failing_commit_cases); - const int working_commit_count = sizeof(passing_commit_cases) / sizeof(*passing_commit_cases); + const int failing_commit_count = ARRAY_SIZE(failing_commit_cases); + const int passing_commit_count = ARRAY_SIZE(passing_commit_cases); int i; - const char *buf; + git_commit *commit; + git_odb_object fake_odb_object; + memset(&fake_odb_object, 0, sizeof(fake_odb_object)); - for (i = 0; i < broken_commit_count; ++i) { - git_commit *commit; + for (i = 0; i < failing_commit_count; ++i) { commit = (git_commit*)git__malloc(sizeof(git_commit)); memset(commit, 0x0, sizeof(git_commit)); commit->object.repo = g_repo; - buf = failing_commit_cases[i]; - cl_git_fail(git_commit__parse(commit, buf, buf + strlen(buf))); + fake_odb_object.buffer = failing_commit_cases[i]; + fake_odb_object.cached.size = strlen(fake_odb_object.buffer); + + cl_git_fail(git_commit__parse(commit, &fake_odb_object)); git_commit__free(commit); } - for (i = 0; i < working_commit_count; ++i) { - git_commit *commit; - + for (i = 0; i < passing_commit_count; ++i) { commit = (git_commit*)git__malloc(sizeof(git_commit)); memset(commit, 0x0, sizeof(git_commit)); commit->object.repo = g_repo; - buf = passing_commit_cases[i]; - cl_git_pass(git_commit__parse(commit, buf, buf + strlen(buf))); + fake_odb_object.buffer = passing_commit_cases[i]; + fake_odb_object.cached.size = strlen(fake_odb_object.buffer); + + cl_git_pass(git_commit__parse(commit, &fake_odb_object)); if (!i) cl_assert_equal_s("", git_commit_message(commit)); From 116bbdf0446cd5335b73e691c3352f368eac9b8f Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 16 Apr 2013 12:08:21 -0700 Subject: [PATCH 17/25] clean up tree pointer casting --- src/tree.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tree.c b/src/tree.c index d6d4b77d3..58eb92f35 100644 --- a/src/tree.c +++ b/src/tree.c @@ -219,16 +219,16 @@ git_tree_entry *git_tree_entry_dup(const git_tree_entry *entry) return copy; } -void git_tree__free(void *tree) +void git_tree__free(void *_tree) { - git_vector *entries = &((git_tree *)tree)->entries; + git_tree *tree = _tree; size_t i; git_tree_entry *e; - git_vector_foreach(entries, i, e) + git_vector_foreach(&tree->entries, i, e) git_tree_entry_free(e); - git_vector_free(entries); + git_vector_free(&tree->entries); git__free(tree); } @@ -372,13 +372,13 @@ static int tree_error(const char *str, const char *path) return -1; } -int git_tree__parse(void *tree, git_odb_object *odb_obj) +int git_tree__parse(void *_tree, git_odb_object *odb_obj) { + git_tree *tree = _tree; const char *buffer = git_odb_object_data(odb_obj); const char *buffer_end = buffer + git_odb_object_size(odb_obj); - git_vector *tree_entries = &((git_tree *)tree)->entries; - if (git_vector_init(tree_entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) + if (git_vector_init(&tree->entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) return -1; while (buffer < buffer_end) { @@ -401,7 +401,7 @@ int git_tree__parse(void *tree, git_odb_object *odb_obj) entry = alloc_entry(buffer); GITERR_CHECK_ALLOC(entry); - if (git_vector_insert(tree_entries, entry) < 0) { + if (git_vector_insert(&tree->entries, entry) < 0) { git__free(entry); return -1; } From 536078688549ac3d50483eecdec5a8169d921927 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 15 Apr 2013 00:09:03 -0700 Subject: [PATCH 18/25] Further threading fixes This builds on the earlier thread safety work to make it so that setting the odb, index, refdb, or config for a repository is done in a threadsafe manner with minimized locking time. This is done by adding a lock to the repository object and using it to guard the assignment of the above listed pointers. The lock is only held to assign the pointer value. This also contains some minor fixes to the other work with pack files to reduce the time that locks are being held to and fix an apparently memory leak. --- src/global.c | 6 ++ src/mwindow.c | 2 +- src/pack.c | 19 ++--- src/pack.h | 2 +- src/refdb_fs.c | 1 - src/repository.c | 214 ++++++++++++++++++++++++++++------------------- src/repository.h | 1 + src/util.h | 21 ++++- 8 files changed, 166 insertions(+), 100 deletions(-) diff --git a/src/global.c b/src/global.c index b7fd8e257..a0571d127 100644 --- a/src/global.c +++ b/src/global.c @@ -135,6 +135,12 @@ int git_threads_init(void) void git_threads_shutdown(void) { + if (_tls_init) { + void *ptr = pthread_getspecific(_tls_key); + pthread_setspecific(_tls_key, NULL); + git__free(ptr); + } + pthread_key_delete(_tls_key); _tls_init = 0; git_mutex_free(&git__mwindow_mutex); diff --git a/src/mwindow.c b/src/mwindow.c index b35503d46..7e5fcdfbc 100644 --- a/src/mwindow.c +++ b/src/mwindow.c @@ -162,7 +162,7 @@ static git_mwindow *new_window( git_mwindow *w; w = git__malloc(sizeof(*w)); - + if (w == NULL) return NULL; diff --git a/src/pack.c b/src/pack.c index 1bffb4778..3a2e7fb5a 100644 --- a/src/pack.c +++ b/src/pack.c @@ -299,29 +299,27 @@ static int pack_index_open(struct git_pack_file *p) int error = 0; size_t name_len, base_len; - if ((error = git_mutex_lock(&p->lock)) < 0) - return error; - if (p->index_map.data) - goto done; + return 0; name_len = strlen(p->pack_name); assert(name_len > strlen(".pack")); /* checked by git_pack_file alloc */ - if ((idx_name = git__malloc(name_len)) == NULL) { - error = -1; - goto done; - } + if ((idx_name = git__malloc(name_len)) == NULL) + return -1; base_len = name_len - strlen(".pack"); memcpy(idx_name, p->pack_name, base_len); memcpy(idx_name + base_len, ".idx", sizeof(".idx")); - error = pack_index_check(idx_name, p); + if ((error = git_mutex_lock(&p->lock)) < 0) + return error; + + if (!p->index_map.data) + error = pack_index_check(idx_name, p); git__free(idx_name); -done: git_mutex_unlock(&p->lock); return error; @@ -820,7 +818,6 @@ void git_packfile_free(struct git_pack_file *p) cache_free(&p->bases); git_mwindow_free_all(&p->mwf); - git_mwindow_file_deregister(&p->mwf); if (p->mwf.fd != -1) p_close(p->mwf.fd); diff --git a/src/pack.h b/src/pack.h index b734ac163..b8014b1ea 100644 --- a/src/pack.h +++ b/src/pack.h @@ -79,7 +79,7 @@ typedef struct { struct git_pack_file { git_mwindow_file mwf; git_map index_map; - git_mutex lock; + git_mutex lock; /* protect updates to mwf and index_map */ uint32_t num_objects; uint32_t num_bad_objects; diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 443871005..742ac6260 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -11,7 +11,6 @@ #include "fileops.h" #include "pack.h" #include "reflog.h" -#include "config.h" #include "refdb.h" #include "refdb_fs.h" diff --git a/src/repository.c b/src/repository.c index cda75b85f..8744b91e6 100644 --- a/src/repository.c +++ b/src/repository.c @@ -32,41 +32,43 @@ #define GIT_TEMPLATE_DIR "/usr/share/git-core/templates" +#define repo_swap_ptr(repo,item_ptr,new_value) \ + git__swap(&repo->lock, (void **)item_ptr, new_value) + static void drop_odb(git_repository *repo) { - if (repo->_odb != NULL) { - GIT_REFCOUNT_OWN(repo->_odb, NULL); - git_odb_free(repo->_odb); - repo->_odb = NULL; + git_odb *dropme = repo_swap_ptr(repo, &repo->_odb, NULL); + if (dropme != NULL) { + GIT_REFCOUNT_OWN(dropme, NULL); + git_odb_free(dropme); } } static void drop_refdb(git_repository *repo) { - if (repo->_refdb != NULL) { - GIT_REFCOUNT_OWN(repo->_refdb, NULL); - git_refdb_free(repo->_refdb); - repo->_refdb = NULL; + git_refdb *dropme = repo_swap_ptr(repo, &repo->_refdb, NULL); + if (dropme != NULL) { + GIT_REFCOUNT_OWN(dropme, NULL); + git_refdb_free(dropme); } } static void drop_config(git_repository *repo) { - if (repo->_config != NULL) { - GIT_REFCOUNT_OWN(repo->_config, NULL); - git_config_free(repo->_config); - repo->_config = NULL; + git_config *dropme = repo_swap_ptr(repo, &repo->_config, NULL); + if (dropme != NULL) { + GIT_REFCOUNT_OWN(dropme, NULL); + git_config_free(dropme); + git_repository__cvar_cache_clear(repo); } - - git_repository__cvar_cache_clear(repo); } static void drop_index(git_repository *repo) { - if (repo->_index != NULL) { - GIT_REFCOUNT_OWN(repo->_index, NULL); - git_index_free(repo->_index); - repo->_index = NULL; + git_index *dropme = repo_swap_ptr(repo, &repo->_index, NULL); + if (dropme != NULL) { + GIT_REFCOUNT_OWN(dropme, NULL); + git_index_free(dropme); } } @@ -79,14 +81,15 @@ void git_repository_free(git_repository *repo) git_attr_cache_flush(repo); git_submodule_config_free(repo); - git__free(repo->path_repository); - git__free(repo->workdir); - drop_config(repo); drop_index(repo); drop_odb(repo); drop_refdb(repo); + git__free(repo->path_repository); + git__free(repo->workdir); + + git_mutex_free(&repo->lock); git__free(repo); } @@ -119,6 +122,8 @@ static git_repository *repository_alloc(void) memset(repo, 0x0, sizeof(git_repository)); + git_mutex_init(&repo->lock); + if (git_cache_init(&repo->objects) < 0) { git__free(repo); return NULL; @@ -549,39 +554,47 @@ on_error: return error; } +static const char *path_unless_empty(git_buf *buf) +{ + return git_buf_len(buf) > 0 ? git_buf_cstr(buf) : NULL; +} + int git_repository_config__weakptr(git_config **out, git_repository *repo) { + int error = 0; + if (repo->_config == NULL) { - git_buf global_buf = GIT_BUF_INIT, xdg_buf = GIT_BUF_INIT, system_buf = GIT_BUF_INIT; - int res; + git_buf global_buf = GIT_BUF_INIT; + git_buf xdg_buf = GIT_BUF_INIT; + git_buf system_buf = GIT_BUF_INIT; + git_config *config; - const char *global_config_path = NULL; - const char *xdg_config_path = NULL; - const char *system_config_path = NULL; + git_config_find_global_r(&global_buf); + git_config_find_xdg_r(&xdg_buf); + git_config_find_system_r(&system_buf); - if (git_config_find_global_r(&global_buf) == 0) - global_config_path = global_buf.ptr; + error = load_config( + &config, repo, + path_unless_empty(&global_buf), + path_unless_empty(&xdg_buf), + path_unless_empty(&system_buf)); + if (!error) { + GIT_REFCOUNT_OWN(config, repo); - if (git_config_find_xdg_r(&xdg_buf) == 0) - xdg_config_path = xdg_buf.ptr; - - if (git_config_find_system_r(&system_buf) == 0) - system_config_path = system_buf.ptr; - - res = load_config(&repo->_config, repo, global_config_path, xdg_config_path, system_config_path); + config = repo_swap_ptr(repo, &repo->_config, config); + if (config != NULL) { + GIT_REFCOUNT_OWN(config, NULL); + git_config_free(config); + } + } git_buf_free(&global_buf); git_buf_free(&xdg_buf); git_buf_free(&system_buf); - - if (res < 0) - return -1; - - GIT_REFCOUNT_OWN(repo->_config, repo); } *out = repo->_config; - return 0; + return error; } int git_repository_config(git_config **out, git_repository *repo) @@ -597,35 +610,46 @@ void git_repository_set_config(git_repository *repo, git_config *config) { assert(repo && config); - drop_config(repo); + GIT_REFCOUNT_OWN(config, repo); + GIT_REFCOUNT_INC(config); - repo->_config = config; - GIT_REFCOUNT_OWN(repo->_config, repo); - GIT_REFCOUNT_INC(repo->_config); + config = repo_swap_ptr(repo, &repo->_config, config); + if (config != NULL) { + GIT_REFCOUNT_OWN(config, NULL); + git_config_free(config); + } + + git_repository__cvar_cache_clear(repo); } int git_repository_odb__weakptr(git_odb **out, git_repository *repo) { + int error = 0; + assert(repo && out); if (repo->_odb == NULL) { git_buf odb_path = GIT_BUF_INIT; - int res; + git_odb *odb; - if (git_buf_joinpath(&odb_path, repo->path_repository, GIT_OBJECTS_DIR) < 0) - return -1; + git_buf_joinpath(&odb_path, repo->path_repository, GIT_OBJECTS_DIR); - res = git_odb_open(&repo->_odb, odb_path.ptr); - git_buf_free(&odb_path); /* done with path */ + error = git_odb_open(&odb, odb_path.ptr); + if (!error) { + GIT_REFCOUNT_OWN(odb, repo); - if (res < 0) - return -1; + odb = repo_swap_ptr(repo, &repo->_odb, odb); + if (odb != NULL) { + GIT_REFCOUNT_OWN(odb, NULL); + git_odb_free(odb); + } + } - GIT_REFCOUNT_OWN(repo->_odb, repo); + git_buf_free(&odb_path); } *out = repo->_odb; - return 0; + return error; } int git_repository_odb(git_odb **out, git_repository *repo) @@ -641,30 +665,39 @@ void git_repository_set_odb(git_repository *repo, git_odb *odb) { assert(repo && odb); - drop_odb(repo); - - repo->_odb = odb; - GIT_REFCOUNT_OWN(repo->_odb, repo); + GIT_REFCOUNT_OWN(odb, repo); GIT_REFCOUNT_INC(odb); + + odb = repo_swap_ptr(repo, &repo->_odb, odb); + if (odb != NULL) { + GIT_REFCOUNT_OWN(odb, NULL); + git_odb_free(odb); + } } int git_repository_refdb__weakptr(git_refdb **out, git_repository *repo) { + int error = 0; + assert(out && repo); if (repo->_refdb == NULL) { - int res; + git_refdb *refdb; - res = git_refdb_open(&repo->_refdb, repo); + error = git_refdb_open(&refdb, repo); + if (!error) { + GIT_REFCOUNT_OWN(refdb, repo); - if (res < 0) - return -1; - - GIT_REFCOUNT_OWN(repo->_refdb, repo); + refdb = repo_swap_ptr(repo, &repo->_refdb, refdb); + if (refdb != NULL) { + GIT_REFCOUNT_OWN(refdb, NULL); + git_refdb_free(refdb); + } + } } *out = repo->_refdb; - return 0; + return error; } int git_repository_refdb(git_refdb **out, git_repository *repo) @@ -678,40 +711,48 @@ int git_repository_refdb(git_refdb **out, git_repository *repo) void git_repository_set_refdb(git_repository *repo, git_refdb *refdb) { - assert (repo && refdb); + assert(repo && refdb); - drop_refdb(repo); - - repo->_refdb = refdb; - GIT_REFCOUNT_OWN(repo->_refdb, repo); + GIT_REFCOUNT_OWN(refdb, repo); GIT_REFCOUNT_INC(refdb); + + refdb = repo_swap_ptr(repo, &repo->_refdb, refdb); + if (refdb != NULL) { + GIT_REFCOUNT_OWN(refdb, NULL); + git_refdb_free(refdb); + } } int git_repository_index__weakptr(git_index **out, git_repository *repo) { + int error = 0; + assert(out && repo); if (repo->_index == NULL) { - int res; git_buf index_path = GIT_BUF_INIT; + git_index *index; - if (git_buf_joinpath(&index_path, repo->path_repository, GIT_INDEX_FILE) < 0) - return -1; + git_buf_joinpath(&index_path, repo->path_repository, GIT_INDEX_FILE); - res = git_index_open(&repo->_index, index_path.ptr); - git_buf_free(&index_path); /* done with path */ + error = git_index_open(&index, index_path.ptr); + if (!error) { + GIT_REFCOUNT_OWN(index, repo); - if (res < 0) - return -1; + index = repo_swap_ptr(repo, &repo->_index, index); + if (index != NULL) { + GIT_REFCOUNT_OWN(index, NULL); + git_index_free(index); + } - GIT_REFCOUNT_OWN(repo->_index, repo); + error = git_index_set_caps(repo->_index, GIT_INDEXCAP_FROM_OWNER); + } - if (git_index_set_caps(repo->_index, GIT_INDEXCAP_FROM_OWNER) < 0) - return -1; + git_buf_free(&index_path); } *out = repo->_index; - return 0; + return error; } int git_repository_index(git_index **out, git_repository *repo) @@ -727,11 +768,14 @@ void git_repository_set_index(git_repository *repo, git_index *index) { assert(repo && index); - drop_index(repo); - - repo->_index = index; - GIT_REFCOUNT_OWN(repo->_index, repo); + GIT_REFCOUNT_OWN(index, repo); GIT_REFCOUNT_INC(index); + + index = repo_swap_ptr(repo, &repo->_index, index); + if (index != NULL) { + GIT_REFCOUNT_OWN(index, NULL); + git_index_free(index); + } } static int check_repositoryformatversion(git_config *config) diff --git a/src/repository.h b/src/repository.h index cc2f8c2b8..873498de0 100644 --- a/src/repository.h +++ b/src/repository.h @@ -83,6 +83,7 @@ struct git_repository { git_refdb *_refdb; git_config *_config; git_index *_index; + git_mutex lock; git_cache objects; git_attr_cache attrcache; diff --git a/src/util.h b/src/util.h index af3ef0b46..a2233a7e8 100644 --- a/src/util.h +++ b/src/util.h @@ -306,11 +306,30 @@ int git__date_parse(git_time_t *out, const char *date); /* * Unescapes a string in-place. - * + * * Edge cases behavior: * - "jackie\" -> "jacky\" * - "chan\\" -> "chan\" */ extern size_t git__unescape(char *str); +/* + * Swap a pointer with thread safety, returning old value. + */ +GIT_INLINE(void *) git__swap(git_mutex *lock, void **ptr_ptr, void *new_ptr) +{ + void *old_ptr; + + if (*ptr_ptr == new_ptr) + return NULL; + if (git_mutex_lock(lock) < 0) + return new_ptr; + + old_ptr = *ptr_ptr; + *ptr_ptr = new_ptr; + + git_mutex_unlock(lock); + return old_ptr; +} + #endif /* INCLUDE_util_h__ */ From e976b56dda6ae3d7d81bd114b61750e97cc918d3 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 15 Apr 2013 14:27:53 -0700 Subject: [PATCH 19/25] Add git__compare_and_swap and use it This removes the lock from the repository object and changes the internals to use the new atomic git__compare_and_swap to update the _odb, _config, _index, and _refdb variables in a threadsafe manner. --- src/global.h | 8 --- src/repository.c | 132 +++++++++++++++++++-------------------------- src/repository.h | 1 - src/thread-utils.h | 41 ++++++++++++++ src/util.h | 19 ------- 5 files changed, 95 insertions(+), 106 deletions(-) diff --git a/src/global.h b/src/global.h index f0ad1df29..badbc0883 100644 --- a/src/global.h +++ b/src/global.h @@ -10,14 +10,6 @@ #include "mwindow.h" #include "hash.h" -#if defined(GIT_THREADS) && defined(_MSC_VER) -# define GIT_MEMORY_BARRIER MemoryBarrier() -#elif defined(GIT_THREADS) -# define GIT_MEMORY_BARRIER __sync_synchronize() -#else -# define GIT_MEMORY_BARRIER /* noop */ -#endif - typedef struct { git_error *last_error; git_error error_t; diff --git a/src/repository.c b/src/repository.c index 8744b91e6..59479dc92 100644 --- a/src/repository.c +++ b/src/repository.c @@ -32,43 +32,57 @@ #define GIT_TEMPLATE_DIR "/usr/share/git-core/templates" -#define repo_swap_ptr(repo,item_ptr,new_value) \ - git__swap(&repo->lock, (void **)item_ptr, new_value) - -static void drop_odb(git_repository *repo) +static void set_odb(git_repository *repo, git_odb *odb) { - git_odb *dropme = repo_swap_ptr(repo, &repo->_odb, NULL); - if (dropme != NULL) { - GIT_REFCOUNT_OWN(dropme, NULL); - git_odb_free(dropme); + if (odb) { + GIT_REFCOUNT_OWN(odb, repo); + GIT_REFCOUNT_INC(odb); + } + + if ((odb = git__swap(repo->_odb, odb)) != NULL) { + GIT_REFCOUNT_OWN(odb, NULL); + git_odb_free(odb); } } -static void drop_refdb(git_repository *repo) +static void set_refdb(git_repository *repo, git_refdb *refdb) { - git_refdb *dropme = repo_swap_ptr(repo, &repo->_refdb, NULL); - if (dropme != NULL) { - GIT_REFCOUNT_OWN(dropme, NULL); - git_refdb_free(dropme); + if (refdb) { + GIT_REFCOUNT_OWN(refdb, repo); + GIT_REFCOUNT_INC(refdb); + } + + if ((refdb = git__swap(repo->_refdb, refdb)) != NULL) { + GIT_REFCOUNT_OWN(refdb, NULL); + git_refdb_free(refdb); } } -static void drop_config(git_repository *repo) +static void set_config(git_repository *repo, git_config *config) { - git_config *dropme = repo_swap_ptr(repo, &repo->_config, NULL); - if (dropme != NULL) { - GIT_REFCOUNT_OWN(dropme, NULL); - git_config_free(dropme); - git_repository__cvar_cache_clear(repo); + if (config) { + GIT_REFCOUNT_OWN(config, repo); + GIT_REFCOUNT_INC(config); } + + if ((config = git__swap(repo->_config, config)) != NULL) { + GIT_REFCOUNT_OWN(config, NULL); + git_config_free(config); + } + + git_repository__cvar_cache_clear(repo); } -static void drop_index(git_repository *repo) +static void set_index(git_repository *repo, git_index *index) { - git_index *dropme = repo_swap_ptr(repo, &repo->_index, NULL); - if (dropme != NULL) { - GIT_REFCOUNT_OWN(dropme, NULL); - git_index_free(dropme); + if (index) { + GIT_REFCOUNT_OWN(index, repo); + GIT_REFCOUNT_INC(index); + } + + if ((index = git__swap(repo->_index, index)) != NULL) { + GIT_REFCOUNT_OWN(index, NULL); + git_index_free(index); } } @@ -81,15 +95,14 @@ void git_repository_free(git_repository *repo) git_attr_cache_flush(repo); git_submodule_config_free(repo); - drop_config(repo); - drop_index(repo); - drop_odb(repo); - drop_refdb(repo); + set_config(repo, NULL); + set_index(repo, NULL); + set_odb(repo, NULL); + set_refdb(repo, NULL); git__free(repo->path_repository); git__free(repo->workdir); - git_mutex_free(&repo->lock); git__free(repo); } @@ -122,8 +135,6 @@ static git_repository *repository_alloc(void) memset(repo, 0x0, sizeof(git_repository)); - git_mutex_init(&repo->lock); - if (git_cache_init(&repo->objects) < 0) { git__free(repo); return NULL; @@ -581,7 +592,7 @@ int git_repository_config__weakptr(git_config **out, git_repository *repo) if (!error) { GIT_REFCOUNT_OWN(config, repo); - config = repo_swap_ptr(repo, &repo->_config, config); + config = git__compare_and_swap(&repo->_config, NULL, config); if (config != NULL) { GIT_REFCOUNT_OWN(config, NULL); git_config_free(config); @@ -609,17 +620,7 @@ int git_repository_config(git_config **out, git_repository *repo) void git_repository_set_config(git_repository *repo, git_config *config) { assert(repo && config); - - GIT_REFCOUNT_OWN(config, repo); - GIT_REFCOUNT_INC(config); - - config = repo_swap_ptr(repo, &repo->_config, config); - if (config != NULL) { - GIT_REFCOUNT_OWN(config, NULL); - git_config_free(config); - } - - git_repository__cvar_cache_clear(repo); + set_config(repo, config); } int git_repository_odb__weakptr(git_odb **out, git_repository *repo) @@ -638,7 +639,7 @@ int git_repository_odb__weakptr(git_odb **out, git_repository *repo) if (!error) { GIT_REFCOUNT_OWN(odb, repo); - odb = repo_swap_ptr(repo, &repo->_odb, odb); + odb = git__compare_and_swap(&repo->_odb, NULL, odb); if (odb != NULL) { GIT_REFCOUNT_OWN(odb, NULL); git_odb_free(odb); @@ -664,15 +665,7 @@ int git_repository_odb(git_odb **out, git_repository *repo) void git_repository_set_odb(git_repository *repo, git_odb *odb) { assert(repo && odb); - - GIT_REFCOUNT_OWN(odb, repo); - GIT_REFCOUNT_INC(odb); - - odb = repo_swap_ptr(repo, &repo->_odb, odb); - if (odb != NULL) { - GIT_REFCOUNT_OWN(odb, NULL); - git_odb_free(odb); - } + set_odb(repo, odb); } int git_repository_refdb__weakptr(git_refdb **out, git_repository *repo) @@ -688,7 +681,7 @@ int git_repository_refdb__weakptr(git_refdb **out, git_repository *repo) if (!error) { GIT_REFCOUNT_OWN(refdb, repo); - refdb = repo_swap_ptr(repo, &repo->_refdb, refdb); + refdb = git__compare_and_swap(&repo->_refdb, NULL, refdb); if (refdb != NULL) { GIT_REFCOUNT_OWN(refdb, NULL); git_refdb_free(refdb); @@ -712,15 +705,7 @@ int git_repository_refdb(git_refdb **out, git_repository *repo) void git_repository_set_refdb(git_repository *repo, git_refdb *refdb) { assert(repo && refdb); - - GIT_REFCOUNT_OWN(refdb, repo); - GIT_REFCOUNT_INC(refdb); - - refdb = repo_swap_ptr(repo, &repo->_refdb, refdb); - if (refdb != NULL) { - GIT_REFCOUNT_OWN(refdb, NULL); - git_refdb_free(refdb); - } + set_refdb(repo, refdb); } int git_repository_index__weakptr(git_index **out, git_repository *repo) @@ -739,7 +724,7 @@ int git_repository_index__weakptr(git_index **out, git_repository *repo) if (!error) { GIT_REFCOUNT_OWN(index, repo); - index = repo_swap_ptr(repo, &repo->_index, index); + index = git__compare_and_swap(&repo->_index, NULL, index); if (index != NULL) { GIT_REFCOUNT_OWN(index, NULL); git_index_free(index); @@ -767,15 +752,7 @@ int git_repository_index(git_index **out, git_repository *repo) void git_repository_set_index(git_repository *repo, git_index *index) { assert(repo && index); - - GIT_REFCOUNT_OWN(index, repo); - GIT_REFCOUNT_INC(index); - - index = repo_swap_ptr(repo, &repo->_index, index); - if (index != NULL) { - GIT_REFCOUNT_OWN(index, NULL); - git_index_free(index); - } + set_index(repo, index); } static int check_repositoryformatversion(git_config *config) @@ -1465,14 +1442,13 @@ static int at_least_one_cb(const char *refname, void *payload) static int repo_contains_no_reference(git_repository *repo) { - int error; - - error = git_reference_foreach(repo, GIT_REF_LISTALL, at_least_one_cb, NULL); + int error = git_reference_foreach(repo, GIT_REF_LISTALL, at_least_one_cb, NULL); if (error == GIT_EUSER) return 0; - - return error == 0 ? 1 : error; + if (!error) + return 1; + return error; } int git_repository_is_empty(git_repository *repo) diff --git a/src/repository.h b/src/repository.h index 873498de0..cc2f8c2b8 100644 --- a/src/repository.h +++ b/src/repository.h @@ -83,7 +83,6 @@ struct git_repository { git_refdb *_refdb; git_config *_config; git_index *_index; - git_mutex lock; git_cache objects; git_attr_cache attrcache; diff --git a/src/thread-utils.h b/src/thread-utils.h index 2ca290adf..7b663182d 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -68,6 +68,21 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a) #endif } +GIT_INLINE(void *) git___compare_and_swap( + volatile void **ptr, void *oldval, void *newval) +{ + bool swapped; +#if defined(GIT_WIN32) + swapped = ((LONGLONG)oldval == InterlockedCompareExchange64( + (LONGLONG volatile *)ptr, (LONGLONG)newval, (LONGLONG)oldval)); +#elif defined(__GNUC__) + swapped = (__sync_val_compare_and_swap(ptr, oldval, newval) == oldval); +#else +# error "Unsupported architecture for atomic operations" +#endif + return swapped ? oldval : newval; +} + #else #define git_thread unsigned int @@ -101,8 +116,34 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a) return --a->val; } +GIT_INLINE(void *) git___compare_and_swap( + volatile void **ptr, void *oldval, void *newval) +{ + if (*ptr == oldval) + *ptr = newval; + else + oldval = newval; + return oldval; +} + #endif +/* Atomically replace oldval with newval + * @return oldval if it was replaced or newval if it was not + */ +#define git__compare_and_swap(P,O,N) \ + git___compare_and_swap((volatile void **)P, O, N) + +#define git__swap(ptr, val) git__compare_and_swap(&ptr, ptr, val) + extern int git_online_cpus(void); +#if defined(GIT_THREADS) && defined(GIT_WIN32) +# define GIT_MEMORY_BARRIER MemoryBarrier() +#elif defined(GIT_THREADS) +# define GIT_MEMORY_BARRIER __sync_synchronize() +#else +# define GIT_MEMORY_BARRIER /* noop */ +#endif + #endif /* INCLUDE_thread_utils_h__ */ diff --git a/src/util.h b/src/util.h index a2233a7e8..82435aee8 100644 --- a/src/util.h +++ b/src/util.h @@ -313,23 +313,4 @@ int git__date_parse(git_time_t *out, const char *date); */ extern size_t git__unescape(char *str); -/* - * Swap a pointer with thread safety, returning old value. - */ -GIT_INLINE(void *) git__swap(git_mutex *lock, void **ptr_ptr, void *new_ptr) -{ - void *old_ptr; - - if (*ptr_ptr == new_ptr) - return NULL; - if (git_mutex_lock(lock) < 0) - return new_ptr; - - old_ptr = *ptr_ptr; - *ptr_ptr = new_ptr; - - git_mutex_unlock(lock); - return old_ptr; -} - #endif /* INCLUDE_util_h__ */ From c628918625c7f779d2050a56998fb2b675f097fb Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 15 Apr 2013 16:31:04 -0700 Subject: [PATCH 20/25] Fixes for Windows cas/threading stuff --- src/thread-utils.h | 9 ++++----- src/win32/pthread.c | 17 +++++++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/thread-utils.h b/src/thread-utils.h index 7b663182d..e53a60c05 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -71,16 +71,15 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a) GIT_INLINE(void *) git___compare_and_swap( volatile void **ptr, void *oldval, void *newval) { - bool swapped; + void *foundval; #if defined(GIT_WIN32) - swapped = ((LONGLONG)oldval == InterlockedCompareExchange64( - (LONGLONG volatile *)ptr, (LONGLONG)newval, (LONGLONG)oldval)); + foundval = InterlockedCompareExchangePointer(ptr, newval, oldval); #elif defined(__GNUC__) - swapped = (__sync_val_compare_and_swap(ptr, oldval, newval) == oldval); + foundval = __sync_val_compare_and_swap(ptr, oldval, newval); #else # error "Unsupported architecture for atomic operations" #endif - return swapped ? oldval : newval; + return (foundval == oldval) ? oldval : newval; } #else diff --git a/src/win32/pthread.c b/src/win32/pthread.c index 105f4b89e..c78213f15 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -14,18 +14,23 @@ int pthread_create( void *GIT_RESTRICT arg) { GIT_UNUSED(attr); - *thread = (pthread_t) CreateThread( + *thread = CreateThread( NULL, 0, (LPTHREAD_START_ROUTINE)start_routine, arg, 0, NULL); return *thread ? 0 : -1; } int pthread_join(pthread_t thread, void **value_ptr) { - int ret; - ret = WaitForSingleObject(thread, INFINITE); - if (ret && value_ptr) - GetExitCodeThread(thread, (void*) value_ptr); - return -(!!ret); + DWORD ret = WaitForSingleObject(thread, INFINITE); + + if (ret == WAIT_OBJECT_0) { + if (value_ptr != NULL) + GetExitCodeThread(thread, (void *)value_ptr); + CloseHandle(thread); + return 0; + } + + return -1; } int pthread_mutex_init(pthread_mutex_t *GIT_RESTRICT mutex, From 38eef6113d8523abfe6746bf727cee2651398ad3 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 16 Apr 2013 14:19:27 -0700 Subject: [PATCH 21/25] Make indexer use shared packfile open code The indexer was creating a packfile object separately from the code in pack.c which was a problem since I put a call to git_mutex_init into just pack.c. This commit updates the pack function for creating a new pack object (i.e. git_packfile_check()) so that it can be used in both places and then makes indexer.c use the shared initialization routine. There are also a few minor formatting and warning message fixes. --- src/indexer.c | 30 ++++++------------------------ src/pack.c | 39 +++++++++++++++++---------------------- src/thread-utils.h | 2 +- src/win32/pthread.c | 5 +++-- src/win32/pthread.h | 11 +++++++---- tests-clar/object/cache.c | 6 +++--- 6 files changed, 37 insertions(+), 56 deletions(-) diff --git a/src/indexer.c b/src/indexer.c index 2cfbd3a5a..50a9d3a37 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -60,36 +60,19 @@ const git_oid *git_indexer_stream_hash(const git_indexer_stream *idx) static int open_pack(struct git_pack_file **out, const char *filename) { - size_t namelen; struct git_pack_file *pack; - struct stat st; - int fd; - namelen = strlen(filename); - pack = git__calloc(1, sizeof(struct git_pack_file) + namelen + 1); - GITERR_CHECK_ALLOC(pack); + if (git_packfile_check(&pack, filename) < 0) + return -1; - memcpy(pack->pack_name, filename, namelen + 1); - - if (p_stat(filename, &st) < 0) { - giterr_set(GITERR_OS, "Failed to stat packfile."); - goto cleanup; - } - - if ((fd = p_open(pack->pack_name, O_RDONLY)) < 0) { + if ((pack->mwf.fd = p_open(pack->pack_name, O_RDONLY)) < 0) { giterr_set(GITERR_OS, "Failed to open packfile."); - goto cleanup; + git_packfile_free(pack); + return -1; } - pack->mwf.fd = fd; - pack->mwf.size = (git_off_t)st.st_size; - *out = pack; return 0; - -cleanup: - git__free(pack); - return -1; } static int parse_header(struct git_pack_header *hdr, struct git_pack_file *pack) @@ -391,7 +374,7 @@ int git_indexer_stream_add(git_indexer_stream *idx, const void *data, size_t siz { int error = -1; struct git_pack_header hdr; - size_t processed; + size_t processed; git_mwindow_file *mwf = &idx->pack->mwf; assert(idx && data && stats); @@ -404,7 +387,6 @@ int git_indexer_stream_add(git_indexer_stream *idx, const void *data, size_t siz /* Make sure we set the new size of the pack */ if (idx->opened_pack) { idx->pack->mwf.size += size; - //printf("\nadding %zu for %zu\n", size, idx->pack->mwf.size); } else { if (open_pack(&idx->pack, idx->pack_file.path_lock) < 0) return -1; diff --git a/src/pack.c b/src/pack.c index 3a2e7fb5a..8e8a01aa9 100644 --- a/src/pack.c +++ b/src/pack.c @@ -794,19 +794,6 @@ git_off_t get_delta_base( * ***********************************************************/ -static struct git_pack_file *packfile_alloc(size_t extra) -{ - struct git_pack_file *p = git__calloc(1, sizeof(*p) + extra); - if (!p) - return NULL; - - p->mwf.fd = -1; - git_mutex_init(&p->lock); - - return p; -} - - void git_packfile_free(struct git_pack_file *p) { if (!p) @@ -902,28 +889,33 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path) { struct stat st; struct git_pack_file *p; - size_t path_len; + size_t path_len = path ? strlen(path) : 0; *pack_out = NULL; - if (!path || (path_len = strlen(path)) <= strlen(".idx")) + if (path_len < strlen(".idx")) return git_odb__error_notfound("invalid packfile path", NULL); - p = packfile_alloc(path_len + 2); + p = git__calloc(1, sizeof(*p) + path_len + 2); GITERR_CHECK_ALLOC(p); + memcpy(p->pack_name, path, path_len + 1); + /* * Make sure a corresponding .pack file exists and that * the index looks sane. */ - path_len -= strlen(".idx"); - memcpy(p->pack_name, path, path_len); + if (git__suffixcmp(path, ".idx") == 0) { + size_t root_len = path_len - strlen(".idx"); - memcpy(p->pack_name + path_len, ".keep", sizeof(".keep")); - if (git_path_exists(p->pack_name) == true) - p->pack_keep = 1; + memcpy(p->pack_name + root_len, ".keep", sizeof(".keep")); + if (git_path_exists(p->pack_name) == true) + p->pack_keep = 1; + + memcpy(p->pack_name + root_len, ".pack", sizeof(".pack")); + path_len = path_len - strlen(".idx") + strlen(".pack"); + } - memcpy(p->pack_name + path_len, ".pack", sizeof(".pack")); if (p_stat(p->pack_name, &st) < 0 || !S_ISREG(st.st_mode)) { git__free(p); return git_odb__error_notfound("packfile not found", NULL); @@ -932,10 +924,13 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path) /* ok, it looks sane as far as we can check without * actually mapping the pack file. */ + p->mwf.fd = -1; p->mwf.size = st.st_size; p->pack_local = 1; p->mtime = (git_time_t)st.st_mtime; + git_mutex_init(&p->lock); + /* see if we can parse the sha1 oid in the packfile name */ if (path_len < 40 || git_oid_fromstr(&p->sha1, path + path_len - GIT_OID_HEXSZ) < 0) diff --git a/src/thread-utils.h b/src/thread-utils.h index e53a60c05..dafe70ad6 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -71,7 +71,7 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a) GIT_INLINE(void *) git___compare_and_swap( volatile void **ptr, void *oldval, void *newval) { - void *foundval; + volatile void *foundval; #if defined(GIT_WIN32) foundval = InterlockedCompareExchangePointer(ptr, newval, oldval); #elif defined(__GNUC__) diff --git a/src/win32/pthread.c b/src/win32/pthread.c index c78213f15..232709e54 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -33,8 +33,9 @@ int pthread_join(pthread_t thread, void **value_ptr) return -1; } -int pthread_mutex_init(pthread_mutex_t *GIT_RESTRICT mutex, - const pthread_mutexattr_t *GIT_RESTRICT mutexattr) +int pthread_mutex_init( + pthread_mutex_t *GIT_RESTRICT mutex, + const pthread_mutexattr_t *GIT_RESTRICT mutexattr) { GIT_UNUSED(mutexattr); InitializeCriticalSection(mutex); diff --git a/src/win32/pthread.h b/src/win32/pthread.h index a219a0137..8277ecf6e 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -25,13 +25,16 @@ typedef HANDLE pthread_cond_t; #define PTHREAD_MUTEX_INITIALIZER {(void*)-1}; -int pthread_create(pthread_t *GIT_RESTRICT, - const pthread_attr_t *GIT_RESTRICT, - void *(*start_routine)(void*), void *__restrict); +int pthread_create( + pthread_t *GIT_RESTRICT, + const pthread_attr_t *GIT_RESTRICT, + void *(*start_routine)(void*), + void *__restrict); int pthread_join(pthread_t, void **); -int pthread_mutex_init(pthread_mutex_t *GIT_RESTRICT, const pthread_mutexattr_t *GIT_RESTRICT); +int pthread_mutex_init( + pthread_mutex_t *GIT_RESTRICT, const pthread_mutexattr_t *GIT_RESTRICT); int pthread_mutex_destroy(pthread_mutex_t *); int pthread_mutex_lock(pthread_mutex_t *); int pthread_mutex_unlock(pthread_mutex_t *); diff --git a/tests-clar/object/cache.c b/tests-clar/object/cache.c index 8121247c9..ed13a6fa8 100644 --- a/tests-clar/object/cache.c +++ b/tests-clar/object/cache.c @@ -213,16 +213,16 @@ void test_object_cache__threadmania(void) fn = (th & 1) ? cache_parsed : cache_raw; #ifdef GIT_THREADS - git_thread_create(&t[th], NULL, fn, data); + cl_git_pass(git_thread_create(&t[th], NULL, fn, data)); #else - fn(data); + cl_assert(fn(data) == data); git__free(data); #endif } #ifdef GIT_THREADS for (th = 0; th < THREADCOUNT; ++th) { - git_thread_join(t[th], &data); + cl_git_pass(git_thread_join(t[th], &data)); cl_assert_equal_i(th, ((int *)data)[0]); git__free(data); } From 5d2d21e536b83ca2cbf8c026b3149fdf776c3f58 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 16 Apr 2013 15:00:43 -0700 Subject: [PATCH 22/25] Consolidate packfile allocation further Rename git_packfile_check to git_packfile_alloc since it is now being used more in that capacity. Fix the various places that use it. Consolidate some repeated code in odb_pack.c related to the allocation of a new pack_backend. --- src/indexer.c | 2 +- src/odb_pack.c | 107 +++++++++++++++++++++++-------------------------- src/pack.c | 2 +- src/pack.h | 3 +- 4 files changed, 55 insertions(+), 59 deletions(-) diff --git a/src/indexer.c b/src/indexer.c index 50a9d3a37..606771927 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -62,7 +62,7 @@ static int open_pack(struct git_pack_file **out, const char *filename) { struct git_pack_file *pack; - if (git_packfile_check(&pack, filename) < 0) + if (git_packfile_alloc(&pack, filename) < 0) return -1; if ((pack->mwf.fd = p_open(pack->pack_name, O_RDONLY)) < 0) { diff --git a/src/odb_pack.c b/src/odb_pack.c index 773e14974..eec79259b 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -207,7 +207,7 @@ static int packfile_load__cb(void *_data, git_buf *path) return 0; } - error = git_packfile_check(&pack, path->ptr); + error = git_packfile_alloc(&pack, path->ptr); if (error == GIT_ENOTFOUND) /* ignore missing .pack file as git does */ return 0; @@ -527,67 +527,17 @@ static void pack_backend__free(git_odb_backend *_backend) git__free(backend); } -int git_odb_backend_one_pack(git_odb_backend **backend_out, const char *idx) +static int pack_backend__alloc(struct pack_backend **out, size_t initial_size) { - struct pack_backend *backend = NULL; - struct git_pack_file *packfile = NULL; - - if (git_packfile_check(&packfile, idx) < 0) - return -1; - - backend = git__calloc(1, sizeof(struct pack_backend)); + struct pack_backend *backend = git__calloc(1, sizeof(struct pack_backend)); GITERR_CHECK_ALLOC(backend); - backend->parent.version = GIT_ODB_BACKEND_VERSION; - if (git_vector_init(&backend->packs, 1, NULL) < 0) - goto on_error; - - if (git_vector_insert(&backend->packs, packfile) < 0) - goto on_error; - - backend->parent.read = &pack_backend__read; - backend->parent.read_prefix = &pack_backend__read_prefix; - backend->parent.read_header = &pack_backend__read_header; - backend->parent.exists = &pack_backend__exists; - backend->parent.refresh = &pack_backend__refresh; - backend->parent.foreach = &pack_backend__foreach; - backend->parent.free = &pack_backend__free; - - *backend_out = (git_odb_backend *)backend; - - return 0; - -on_error: - git_vector_free(&backend->packs); - git__free(backend); - git__free(packfile); - return -1; -} - -int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) -{ - struct pack_backend *backend = NULL; - git_buf path = GIT_BUF_INIT; - - backend = git__calloc(1, sizeof(struct pack_backend)); - GITERR_CHECK_ALLOC(backend); - backend->parent.version = GIT_ODB_BACKEND_VERSION; - - if (git_vector_init(&backend->packs, 8, packfile_sort__cb) < 0 || - git_buf_joinpath(&path, objects_dir, "pack") < 0) - { + if (git_vector_init(&backend->packs, initial_size, packfile_sort__cb) < 0) { git__free(backend); return -1; } - if (git_path_isdir(git_buf_cstr(&path)) == true) { - int error; - - backend->pack_folder = git_buf_detach(&path); - error = pack_backend__refresh((git_odb_backend *)backend); - if (error < 0) - return error; - } + backend->parent.version = GIT_ODB_BACKEND_VERSION; backend->parent.read = &pack_backend__read; backend->parent.read_prefix = &pack_backend__read_prefix; @@ -598,9 +548,54 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) backend->parent.writepack = &pack_backend__writepack; backend->parent.free = &pack_backend__free; + *out = backend; + return 0; +} + +int git_odb_backend_one_pack(git_odb_backend **backend_out, const char *idx) +{ + struct pack_backend *backend = NULL; + struct git_pack_file *packfile = NULL; + + if (pack_backend__alloc(&backend, 1) < 0) + return -1; + + if (git_packfile_alloc(&packfile, idx) < 0 || + git_vector_insert(&backend->packs, packfile) < 0) + { + pack_backend__free((git_odb_backend *)backend); + return -1; + } + + *backend_out = (git_odb_backend *)backend; + return 0; +} + +int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) +{ + int error = 0; + struct pack_backend *backend = NULL; + git_buf path = GIT_BUF_INIT; + + if (pack_backend__alloc(&backend, 8) < 0) + return -1; + + if (!(error = git_buf_joinpath(&path, objects_dir, "pack")) && + git_path_isdir(git_buf_cstr(&path))) + { + backend->pack_folder = git_buf_detach(&path); + + error = pack_backend__refresh((git_odb_backend *)backend); + } + + if (error < 0) { + pack_backend__free((git_odb_backend *)backend); + backend = NULL; + } + *backend_out = (git_odb_backend *)backend; git_buf_free(&path); - return 0; + return error; } diff --git a/src/pack.c b/src/pack.c index 8e8a01aa9..33cdf760a 100644 --- a/src/pack.c +++ b/src/pack.c @@ -885,7 +885,7 @@ cleanup: return -1; } -int git_packfile_check(struct git_pack_file **pack_out, const char *path) +int git_packfile_alloc(struct git_pack_file **pack_out, const char *path) { struct stat st; struct git_pack_file *p; diff --git a/src/pack.h b/src/pack.h index b8014b1ea..aeeac9ce1 100644 --- a/src/pack.h +++ b/src/pack.h @@ -143,7 +143,8 @@ git_off_t get_delta_base(struct git_pack_file *p, git_mwindow **w_curs, git_off_t delta_obj_offset); void git_packfile_free(struct git_pack_file *p); -int git_packfile_check(struct git_pack_file **pack_out, const char *path); +int git_packfile_alloc(struct git_pack_file **pack_out, const char *path); + int git_pack_entry_find( struct git_pack_entry *e, struct git_pack_file *p, From 865e2dd4440596d7cd874556a694eea30336c69b Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 17 Apr 2013 23:58:37 +0200 Subject: [PATCH 23/25] tests: Cleanup commit parse testing code --- tests-clar/commit/parse.c | 45 ++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index ad2c746ca..8de4401dc 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -264,37 +264,40 @@ gpgsig -----BEGIN PGP SIGNATURE-----\n\ a simple commit which works\n", }; +static int parse_commit(git_commit **out, const char *buffer) +{ + git_commit *commit; + git_odb_object fake_odb_object; + int error; + + commit = (git_commit*)git__malloc(sizeof(git_commit)); + memset(commit, 0x0, sizeof(git_commit)); + commit->object.repo = g_repo; + + memset(&fake_odb_object, 0x0, sizeof(git_odb_object)); + fake_odb_object.buffer = (char *)buffer; + fake_odb_object.cached.size = strlen(fake_odb_object.buffer); + + error = git_commit__parse(commit, &fake_odb_object); + + *out = commit; + return error; +} + void test_commit_parse__entire_commit(void) { const int failing_commit_count = ARRAY_SIZE(failing_commit_cases); const int passing_commit_count = ARRAY_SIZE(passing_commit_cases); int i; git_commit *commit; - git_odb_object fake_odb_object; - memset(&fake_odb_object, 0, sizeof(fake_odb_object)); for (i = 0; i < failing_commit_count; ++i) { - commit = (git_commit*)git__malloc(sizeof(git_commit)); - memset(commit, 0x0, sizeof(git_commit)); - commit->object.repo = g_repo; - - fake_odb_object.buffer = failing_commit_cases[i]; - fake_odb_object.cached.size = strlen(fake_odb_object.buffer); - - cl_git_fail(git_commit__parse(commit, &fake_odb_object)); - + cl_git_fail(parse_commit(&commit, failing_commit_cases[i])); git_commit__free(commit); } for (i = 0; i < passing_commit_count; ++i) { - commit = (git_commit*)git__malloc(sizeof(git_commit)); - memset(commit, 0x0, sizeof(git_commit)); - commit->object.repo = g_repo; - - fake_odb_object.buffer = passing_commit_cases[i]; - fake_odb_object.cached.size = strlen(fake_odb_object.buffer); - - cl_git_pass(git_commit__parse(commit, &fake_odb_object)); + cl_git_pass(parse_commit(&commit, passing_commit_cases[i])); if (!i) cl_assert_equal_s("", git_commit_message(commit)); @@ -387,9 +390,7 @@ This commit has a few LF at the start of the commit message"; memset(commit, 0x0, sizeof(git_commit)); commit->object.repo = g_repo; - cl_git_pass(git_commit__parse_buffer(commit, buffer, strlen(buffer))); - + cl_git_pass(parse_commit(&commit, buffer)); cl_assert_equal_s(message, git_commit_message(commit)); - git_commit__free(commit); } From cf9709b64eb49d5a0ee1181b80c950e518fb45b0 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Mon, 22 Apr 2013 16:53:46 +0200 Subject: [PATCH 24/25] tests: Do not warn for unused variable --- tests-clar/object/cache.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests-clar/object/cache.c b/tests-clar/object/cache.c index ed13a6fa8..b36bf2726 100644 --- a/tests-clar/object/cache.c +++ b/tests-clar/object/cache.c @@ -195,10 +195,13 @@ static void *cache_raw(void *arg) void test_object_cache__threadmania(void) { int try, th, max_i; - git_thread t[THREADCOUNT]; void *data; void *(*fn)(void *); +#ifdef GIT_THREADS + git_thread t[THREADCOUNT]; +#endif + for (max_i = 0; g_data[max_i].sha != NULL; ++max_i) /* count up */; From d87715926049390a2417a2476742114ec966686a Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Mon, 22 Apr 2013 17:04:52 +0200 Subject: [PATCH 25/25] cache: Max cache size, and evict when the cache fills up --- include/git2/common.h | 3 ++- src/cache.c | 25 ++++++++++++++++++------- src/cache.h | 1 + src/util.c | 6 +++++- tests-clar/object/cache.c | 6 +++--- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index 80d83d345..ccd252fda 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -131,7 +131,8 @@ enum { GIT_OPT_SET_MWINDOW_MAPPED_LIMIT, GIT_OPT_GET_SEARCH_PATH, GIT_OPT_SET_SEARCH_PATH, - GIT_OPT_SET_CACHE_LIMIT, + GIT_OPT_SET_CACHE_OBJECT_LIMIT, + GIT_OPT_SET_CACHE_MAX_SIZE, GIT_OPT_ENABLE_CACHING }; diff --git a/src/cache.c b/src/cache.c index c51be895e..ca122fb77 100644 --- a/src/cache.c +++ b/src/cache.c @@ -18,6 +18,7 @@ GIT__USE_OIDMAP bool git_cache__enabled = true; +size_t git_cache__max_storage = (4 * 1024 * 1024); static size_t git_cache__max_object_size[8] = { 0, /* GIT_OBJ__EXT1 */ @@ -70,19 +71,25 @@ int git_cache_init(git_cache *cache) return 0; } -void git_cache_clear(git_cache *cache) +/* called with lock */ +static void clear_cache(git_cache *cache) { git_cached_obj *evict = NULL; - if (git_mutex_lock(&cache->lock) < 0) - return; - kh_foreach_value(cache->map, evict, { git_cached_obj_decref(evict); }); kh_clear(oid, cache->map); cache->used_memory = 0; +} + +void git_cache_clear(git_cache *cache) +{ + if (git_mutex_lock(&cache->lock) < 0) + return; + + clear_cache(cache); git_mutex_unlock(&cache->lock); } @@ -95,14 +102,15 @@ void git_cache_free(git_cache *cache) git_mutex_free(&cache->lock); } -/* Call with lock, yo */ -static void cache_evict_entries(git_cache *cache, size_t evict_count) +/* Called with lock */ +static void cache_evict_entries(git_cache *cache) { uint32_t seed = rand(); + size_t evict_count = 8; /* do not infinite loop if there's not enough entries to evict */ if (evict_count > kh_size(cache->map)) { - git_cache_clear(cache); + clear_cache(cache); return; } @@ -163,6 +171,9 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) if (git_mutex_lock(&cache->lock) < 0) return entry; + if (cache->used_memory > git_cache__max_storage) + cache_evict_entries(cache); + pos = kh_get(oid, cache->map, &entry->oid); /* not found */ diff --git a/src/cache.h b/src/cache.h index e95d521fe..1715c7220 100644 --- a/src/cache.h +++ b/src/cache.h @@ -35,6 +35,7 @@ typedef struct { } git_cache; extern bool git_cache__enabled; +extern size_t git_cache__max_storage; int git_cache_set_max_object_size(git_otype type, size_t size); diff --git a/src/util.c b/src/util.c index 1ed5d5d16..c3fc69756 100644 --- a/src/util.c +++ b/src/util.c @@ -95,7 +95,7 @@ int git_libgit2_opts(int key, ...) error = git_futils_dirs_set(error, va_arg(ap, const char *)); break; - case GIT_OPT_SET_CACHE_LIMIT: + case GIT_OPT_SET_CACHE_OBJECT_LIMIT: { git_otype type = (git_otype)va_arg(ap, int); size_t size = va_arg(ap, size_t); @@ -103,6 +103,10 @@ int git_libgit2_opts(int key, ...) break; } + case GIT_OPT_SET_CACHE_MAX_SIZE: + git_cache__max_storage = va_arg(ap, size_t); + break; + case GIT_OPT_ENABLE_CACHING: git_cache__enabled = (va_arg(ap, int) != 0); break; diff --git a/tests-clar/object/cache.c b/tests-clar/object/cache.c index b36bf2726..a3eba8737 100644 --- a/tests-clar/object/cache.c +++ b/tests-clar/object/cache.c @@ -13,7 +13,7 @@ void test_object_cache__cleanup(void) git_repository_free(g_repo); g_repo = NULL; - git_libgit2_opts(GIT_OPT_SET_CACHE_LIMIT, (int)GIT_OBJ_BLOB, (size_t)0); + git_libgit2_opts(GIT_OPT_SET_CACHE_OBJECT_LIMIT, (int)GIT_OBJ_BLOB, (size_t)0); } static struct { @@ -54,7 +54,7 @@ void test_object_cache__cache_everything(void) git_odb *odb; git_libgit2_opts( - GIT_OPT_SET_CACHE_LIMIT, (int)GIT_OBJ_BLOB, (size_t)32767); + GIT_OPT_SET_CACHE_OBJECT_LIMIT, (int)GIT_OBJ_BLOB, (size_t)32767); cl_git_pass(git_repository_odb(&odb, g_repo)); @@ -103,7 +103,7 @@ void test_object_cache__cache_no_blobs(void) git_object *obj; git_odb *odb; - git_libgit2_opts(GIT_OPT_SET_CACHE_LIMIT, (int)GIT_OBJ_BLOB, (size_t)0); + git_libgit2_opts(GIT_OPT_SET_CACHE_OBJECT_LIMIT, (int)GIT_OBJ_BLOB, (size_t)0); cl_git_pass(git_repository_odb(&odb, g_repo));