From b8ab782a6dc206101d78852036a8e86d5b812278 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Feb 2017 11:43:32 +0100 Subject: [PATCH 1/3] attrcache: do not lock/unlock the mutex directly Improve encapsulation by not referencing the attrcache mutex directly but instead using the `attr_cache_lock` and `attr_cache_unlock` functions. --- src/attrcache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/attrcache.c b/src/attrcache.c index 6cac14bc3..d460db80c 100644 --- a/src/attrcache.c +++ b/src/attrcache.c @@ -309,7 +309,7 @@ static void attr_cache__free(git_attr_cache *cache) if (!cache) return; - unlock = (git_mutex_lock(&cache->lock) == 0); + unlock = (attr_cache_lock(cache) == 0); if (cache->files != NULL) { git_attr_file_entry *entry; @@ -345,7 +345,7 @@ static void attr_cache__free(git_attr_cache *cache) cache->cfg_excl_file = NULL; if (unlock) - git_mutex_unlock(&cache->lock); + attr_cache_unlock(cache); git_mutex_free(&cache->lock); git__free(cache); @@ -429,7 +429,7 @@ int git_attr_cache__insert_macro(git_repository *repo, git_attr_rule *macro) if (macro->assigns.length == 0) return 0; - if (git_mutex_lock(&cache->lock) < 0) { + if (attr_cache_lock(cache) < 0) { giterr_set(GITERR_OS, "unable to get attr cache lock"); error = -1; } else { From c11510103d9510f1a4b6e3da90464bcef52250c9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Feb 2017 11:52:15 +0100 Subject: [PATCH 2/3] attrcache: replace existing file entry with `git__swap` When doing an upsert of a file, we used to use `git__compare_and_swap`, comparing the entry's file which is to be replaced with itself. This can be more easily formulated by using `git__swap`, which unconditionally replaces the value. --- src/attrcache.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/attrcache.c b/src/attrcache.c index d460db80c..1812376ea 100644 --- a/src/attrcache.c +++ b/src/attrcache.c @@ -103,8 +103,11 @@ static int attr_cache_upsert(git_attr_cache *cache, git_attr_file *file) GIT_REFCOUNT_OWN(file, entry); GIT_REFCOUNT_INC(file); - old = git__compare_and_swap( - &entry->file[file->source], entry->file[file->source], file); + /* + * Replace the existing value if another thread has + * created it in the meantime. + */ + old = git__swap(entry->file[file->source], file); if (old) { GIT_REFCOUNT_OWN(old, NULL); From ce6f61daf0e1985d615e640102b5496ff92e401c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Feb 2017 15:14:04 +0100 Subject: [PATCH 3/3] attrcache: remove useless `do_init` indirection Remove useless indirection from `git_attr_cache__init` to `git_attr_cache__do_init`. The difference is that the `git_attr_cache__init` macro first checks if the cache is already initialized and, if so, not call `git_attr_cache__do_init`. But actually, `git_attr_cache__do_init` already does the same thing and returns immediately if the cache is already initialized. Remove the indirection. --- src/attrcache.c | 2 +- src/attrcache.h | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/attrcache.c b/src/attrcache.c index 1812376ea..4df14ee2c 100644 --- a/src/attrcache.c +++ b/src/attrcache.c @@ -354,7 +354,7 @@ static void attr_cache__free(git_attr_cache *cache) git__free(cache); } -int git_attr_cache__do_init(git_repository *repo) +int git_attr_cache__init(git_repository *repo) { int ret = 0; git_attr_cache *cache = git_repository_attr_cache(repo); diff --git a/src/attrcache.h b/src/attrcache.h index 44e1ffdce..b91edd3e8 100644 --- a/src/attrcache.h +++ b/src/attrcache.h @@ -22,10 +22,7 @@ typedef struct { git_pool pool; } git_attr_cache; -extern int git_attr_cache__do_init(git_repository *repo); - -#define git_attr_cache__init(REPO) \ - (git_repository_attr_cache(REPO) ? 0 : git_attr_cache__do_init(REPO)) +extern int git_attr_cache__init(git_repository *repo); /* get file - loading and reload as needed */ extern int git_attr_cache__get(