summaryrefslogtreecommitdiff
path: root/builtin
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2020-02-14 20:54:20 (GMT)
committerJunio C Hamano <gitster@pobox.com>2020-02-14 20:54:20 (GMT)
commit56ceb64eb0377acb2884226d4fb7d7c3f4032354 (patch)
tree3ab8775533954b285d1abf67d2ea0f53c7149154 /builtin
parent0da63da7942ce810126cd54d9f72a46f3d616895 (diff)
parentf1928f04b2510c522c25d482e6ab4a17077add9a (diff)
downloadgit-56ceb64eb0377acb2884226d4fb7d7c3f4032354.zip
git-56ceb64eb0377acb2884226d4fb7d7c3f4032354.tar.gz
git-56ceb64eb0377acb2884226d4fb7d7c3f4032354.tar.bz2
Merge branch 'mt/threaded-grep-in-object-store'
Traditionally, we avoided threaded grep while searching in objects (as opposed to files in the working tree) as accesses to the object layer is not thread-safe. This limitation is getting lifted. * mt/threaded-grep-in-object-store: grep: use no. of cores as the default no. of threads grep: move driver pre-load out of critical section grep: re-enable threads in non-worktree case grep: protect packed_git [re-]initialization grep: allow submodule functions to run in parallel submodule-config: add skip_if_read option to repo_read_gitmodules() grep: replace grep_read_mutex by internal obj read lock object-store: allow threaded access to object reading replace-object: make replace operations thread-safe grep: fix racy calls in grep_objects() grep: fix race conditions at grep_submodule() grep: fix race conditions on userdiff calls
Diffstat (limited to 'builtin')
-rw-r--r--builtin/grep.c93
1 files changed, 46 insertions, 47 deletions
diff --git a/builtin/grep.c b/builtin/grep.c
index ae2d5bb..99e2685 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -24,6 +24,7 @@
#include "submodule.h"
#include "submodule-config.h"
#include "object-store.h"
+#include "packfile.h"
static char const * const grep_usage[] = {
N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
@@ -32,7 +33,6 @@ static char const * const grep_usage[] = {
static int recurse_submodules;
-#define GREP_NUM_THREADS_DEFAULT 8
static int num_threads;
static pthread_t *threads;
@@ -91,8 +91,11 @@ static pthread_cond_t cond_result;
static int skip_first_line;
-static void add_work(struct grep_opt *opt, const struct grep_source *gs)
+static void add_work(struct grep_opt *opt, struct grep_source *gs)
{
+ if (opt->binary != GREP_BINARY_TEXT)
+ grep_source_load_driver(gs, opt->repo->index);
+
grep_lock();
while ((todo_end+1) % ARRAY_SIZE(todo) == todo_done) {
@@ -100,9 +103,6 @@ static void add_work(struct grep_opt *opt, const struct grep_source *gs)
}
todo[todo_end].source = *gs;
- if (opt->binary != GREP_BINARY_TEXT)
- grep_source_load_driver(&todo[todo_end].source,
- opt->repo->index);
todo[todo_end].done = 0;
strbuf_reset(&todo[todo_end].out);
todo_end = (todo_end + 1) % ARRAY_SIZE(todo);
@@ -200,12 +200,12 @@ static void start_threads(struct grep_opt *opt)
int i;
pthread_mutex_init(&grep_mutex, NULL);
- pthread_mutex_init(&grep_read_mutex, NULL);
pthread_mutex_init(&grep_attr_mutex, NULL);
pthread_cond_init(&cond_add, NULL);
pthread_cond_init(&cond_write, NULL);
pthread_cond_init(&cond_result, NULL);
grep_use_locks = 1;
+ enable_obj_read_lock();
for (i = 0; i < ARRAY_SIZE(todo); i++) {
strbuf_init(&todo[i].out, 0);
@@ -257,12 +257,12 @@ static int wait_all(void)
free(threads);
pthread_mutex_destroy(&grep_mutex);
- pthread_mutex_destroy(&grep_read_mutex);
pthread_mutex_destroy(&grep_attr_mutex);
pthread_cond_destroy(&cond_add);
pthread_cond_destroy(&cond_write);
pthread_cond_destroy(&cond_result);
grep_use_locks = 0;
+ disable_obj_read_lock();
return hit;
}
@@ -295,16 +295,6 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
return st;
}
-static void *lock_and_read_oid_file(const struct object_id *oid, enum object_type *type, unsigned long *size)
-{
- void *data;
-
- grep_read_lock();
- data = read_object_file(oid, type, size);
- grep_read_unlock();
- return data;
-}
-
static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
const char *filename, int tree_name_len,
const char *path)
@@ -407,30 +397,28 @@ static int grep_submodule(struct grep_opt *opt,
{
struct repository subrepo;
struct repository *superproject = opt->repo;
- const struct submodule *sub = submodule_from_path(superproject,
- &null_oid, path);
+ const struct submodule *sub;
struct grep_opt subopt;
int hit;
- /*
- * NEEDSWORK: submodules functions need to be protected because they
- * access the object store via config_from_gitmodules(): the latter
- * uses get_oid() which, for now, relies on the global the_repository
- * object.
- */
- grep_read_lock();
+ sub = submodule_from_path(superproject, &null_oid, path);
- if (!is_submodule_active(superproject, path)) {
- grep_read_unlock();
+ if (!is_submodule_active(superproject, path))
return 0;
- }
- if (repo_submodule_init(&subrepo, superproject, sub)) {
- grep_read_unlock();
+ if (repo_submodule_init(&subrepo, superproject, sub))
return 0;
- }
- repo_read_gitmodules(&subrepo);
+ /*
+ * NEEDSWORK: repo_read_gitmodules() might call
+ * add_to_alternates_memory() via config_from_gitmodules(). This
+ * operation causes a race condition with concurrent object readings
+ * performed by the worker threads. That's why we need obj_read_lock()
+ * here. It should be removed once it's no longer necessary to add the
+ * subrepo's odbs to the in-memory alternates list.
+ */
+ obj_read_lock();
+ repo_read_gitmodules(&subrepo, 0);
/*
* NEEDSWORK: This adds the submodule's object directory to the list of
@@ -443,7 +431,7 @@ static int grep_submodule(struct grep_opt *opt,
* object.
*/
add_to_alternates_memory(subrepo.objects->odb->path);
- grep_read_unlock();
+ obj_read_unlock();
memcpy(&subopt, opt, sizeof(subopt));
subopt.repo = &subrepo;
@@ -455,14 +443,12 @@ static int grep_submodule(struct grep_opt *opt,
unsigned long size;
struct strbuf base = STRBUF_INIT;
+ obj_read_lock();
object = parse_object_or_die(oid, oid_to_hex(oid));
-
- grep_read_lock();
+ obj_read_unlock();
data = read_object_with_reference(&subrepo,
&object->oid, tree_type,
&size, NULL);
- grep_read_unlock();
-
if (!data)
die(_("unable to read tree (%s)"), oid_to_hex(&object->oid));
@@ -587,7 +573,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
void *data;
unsigned long size;
- data = lock_and_read_oid_file(&entry.oid, &type, &size);
+ data = read_object_file(&entry.oid, &type, &size);
if (!data)
die(_("unable to read tree (%s)"),
oid_to_hex(&entry.oid));
@@ -625,12 +611,9 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
struct strbuf base;
int hit, len;
- grep_read_lock();
data = read_object_with_reference(opt->repo,
&obj->oid, tree_type,
&size, NULL);
- grep_read_unlock();
-
if (!data)
die(_("unable to read tree (%s)"), oid_to_hex(&obj->oid));
@@ -659,13 +642,18 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
for (i = 0; i < nr; i++) {
struct object *real_obj;
+
+ obj_read_lock();
real_obj = deref_tag(opt->repo, list->objects[i].item,
NULL, 0);
+ obj_read_unlock();
/* load the gitmodules file for this rev */
if (recurse_submodules) {
submodule_free(opt->repo);
+ obj_read_lock();
gitmodules_config_oid(&real_obj->oid);
+ obj_read_unlock();
}
if (grep_object(opt, pathspec, real_obj, list->objects[i].name,
list->objects[i].path)) {
@@ -1065,7 +1053,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pathspec.recursive = 1;
pathspec.recurse_submodules = !!recurse_submodules;
- if (list.nr || cached || show_in_pager) {
+ if (recurse_submodules && untracked)
+ die(_("--untracked not supported with --recurse-submodules"));
+
+ if (show_in_pager) {
if (num_threads > 1)
warning(_("invalid option combination, ignoring --threads"));
num_threads = 1;
@@ -1075,7 +1066,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
} else if (num_threads < 0)
die(_("invalid number of threads specified (%d)"), num_threads);
else if (num_threads == 0)
- num_threads = HAVE_THREADS ? GREP_NUM_THREADS_DEFAULT : 1;
+ num_threads = HAVE_THREADS ? online_cpus() : 1;
if (num_threads > 1) {
if (!HAVE_THREADS)
@@ -1084,6 +1075,17 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
&& (opt.pre_context || opt.post_context ||
opt.file_break || opt.funcbody))
skip_first_line = 1;
+
+ /*
+ * Pre-read gitmodules (if not read already) and force eager
+ * initialization of packed_git to prevent racy lazy
+ * reading/initialization once worker threads are started.
+ */
+ if (recurse_submodules)
+ repo_read_gitmodules(the_repository, 1);
+ if (startup_info->have_repository)
+ (void)get_packed_git(the_repository);
+
start_threads(&opt);
} else {
/*
@@ -1118,9 +1120,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
}
}
- if (recurse_submodules && untracked)
- die(_("--untracked not supported with --recurse-submodules"));
-
if (!show_in_pager && !opt.status_only)
setup_pager();