"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: speed up 'got status' a little bit
To:
gameoftrees@openbsd.org
Date:
Thu, 30 Sep 2021 17:23:32 +0200

Download raw body.

Thread
On Thu, Sep 30, 2021 at 05:03:57PM +0200, Stefan Sperling wrote:
> Existing regression tests keep passing. Because this is just about a
> performance boost there is no need for a new test in the regress suite.

There is one problem, however.

Many operations must now disable ignore list processing.
Paths which are already under version control and happen to match an
ignore pattern would otherwise be skipped. Yes, it is possible to
have versioned files inside an ignored directory. There is nothing
we can do to prevent this. And ignore lists are supposed to match on
unversioned files only.

Fix this by setting 'no_ignores' for all such cases, which should have
been done in the first place. We did the same for 'got rebase' some time
ago because Tracey found files missing after using 'got rebase' combined
with an ignore rule that matched some versioned files being rebased.

Fixed patch:

diff 78caff98cb8783be4d83da7279e407d4ccd1f187 /home/stsp/src/got
blob - aec06af5925a0cd63ffc6b150466bad036d5abd8
file + lib/fileindex.c
--- lib/fileindex.c
+++ lib/fileindex.c
@@ -991,7 +991,7 @@ static const struct got_error *
 walk_dir(struct got_pathlist_entry **next, struct got_fileindex *fileindex,
     struct got_fileindex_entry **ie, struct got_pathlist_entry *dle, int fd,
     const char *path, const char *rootpath, struct got_repository *repo,
-    struct got_fileindex_diff_dir_cb *cb, void *cb_arg)
+    int ignore, struct got_fileindex_diff_dir_cb *cb, void *cb_arg)
 {
 	const struct got_error *err = NULL;
 	struct dirent *de = dle->data;
@@ -1013,7 +1013,7 @@ walk_dir(struct got_pathlist_entry **next, struct got_
 	} else
 		type = de->d_type;
 
-	if (type == DT_DIR) {
+	if (type == DT_DIR && !ignore) {
 		char *subpath;
 		char *subdirpath;
 		struct got_pathlist_head subdirlist;
@@ -1079,6 +1079,7 @@ diff_fileindex_dir(struct got_fileindex *fileindex,
 	struct dirent *de = NULL;
 	size_t path_len = strlen(path);
 	struct got_pathlist_entry *dle;
+	int ignore;
 
 	if (cb->diff_traverse) {
 		err = cb->diff_traverse(cb_arg, path, dirfd);
@@ -1108,18 +1109,19 @@ diff_fileindex_dir(struct got_fileindex *fileindex,
 					break;
 				*ie = walk_fileindex(fileindex, *ie);
 				err = walk_dir(&dle, fileindex, ie, dle, dirfd,
-				    path, rootpath, repo, cb, cb_arg);
+				    path, rootpath, repo, 0, cb, cb_arg);
 			} else if (cmp < 0 ) {
 				err = cb->diff_old(cb_arg, *ie, path);
 				if (err)
 					break;
 				*ie = walk_fileindex(fileindex, *ie);
 			} else {
-				err = cb->diff_new(cb_arg, de, path, dirfd);
+				err = cb->diff_new(&ignore, cb_arg, de, path,
+				    dirfd);
 				if (err)
 					break;
 				err = walk_dir(&dle, fileindex, ie, dle, dirfd,
-				    path, rootpath, repo, cb, cb_arg);
+				    path, rootpath, repo, ignore, cb, cb_arg);
 			}
 			if (err)
 				break;
@@ -1130,11 +1132,11 @@ diff_fileindex_dir(struct got_fileindex *fileindex,
 			*ie = walk_fileindex(fileindex, *ie);
 		} else if (dle) {
 			de = dle->data;
-			err = cb->diff_new(cb_arg, de, path, dirfd);
+			err = cb->diff_new(&ignore, cb_arg, de, path, dirfd);
 			if (err)
 				break;
 			err = walk_dir(&dle, fileindex, ie, dle, dirfd, path,
-			    rootpath, repo, cb, cb_arg);
+			    rootpath, repo, ignore, cb, cb_arg);
 			if (err)
 				break;
 		}
blob - d03058e83c5f5c0a69baf66795455eb3ab8b7af7
file + lib/got_lib_fileindex.h
--- lib/got_lib_fileindex.h
+++ lib/got_lib_fileindex.h
@@ -148,7 +148,7 @@ typedef const struct got_error *(*got_fileindex_diff_d
     struct got_fileindex_entry *, struct dirent *, const char *, int);
 typedef const struct got_error *(*got_fileindex_diff_dir_old_cb)(void *,
     struct got_fileindex_entry *, const char *);
-typedef const struct got_error *(*got_fileindex_diff_dir_new_cb)(void *,
+typedef const struct got_error *(*got_fileindex_diff_dir_new_cb)(int *, void *,
     struct dirent *, const char *, int);
 typedef const struct got_error *(*got_fileindex_diff_dir_traverse)(void *,
     const char *, int);
blob - 2fab18710a8a5331db77b19986086dd87bc4820d
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -3617,12 +3617,16 @@ add_ignores(struct got_pathlist_head *ignores, const c
 }
 
 static const struct got_error *
-status_new(void *arg, struct dirent *de, const char *parent_path, int dirfd)
+status_new(int *ignore, void *arg, struct dirent *de, const char *parent_path,
+    int dirfd)
 {
 	const struct got_error *err = NULL;
 	struct diff_dir_cb_arg *a = arg;
 	char *path = NULL;
 
+	if (ignore != NULL)
+		*ignore = 0;
+
 	if (a->cancel_cb && a->cancel_cb(a->cancel_arg))
 		return got_error(GOT_ERR_CANCELLED);
 
@@ -3633,9 +3637,12 @@ status_new(void *arg, struct dirent *de, const char *p
 		path = de->d_name;
 	}
 
-	if (de->d_type != DT_DIR &&
-	    got_path_is_child(path, a->status_path, a->status_path_len)
-	    && !match_ignores(a->ignores, path))
+	if (de->d_type == DT_DIR) {
+		if (!a->no_ignores && ignore != NULL &&
+		    match_ignores(a->ignores, path))
+			*ignore = 1;
+	} else if (!match_ignores(a->ignores, path) &&
+	    got_path_is_child(path, a->status_path, a->status_path_len))
 		err = (*a->status_cb)(a->status_arg, GOT_STATUS_UNVERSIONED,
 		    GOT_STATUS_NO_CHANGE, path, NULL, NULL, NULL, -1, NULL);
 	if (parent_path[0])
@@ -4213,7 +4220,7 @@ got_worktree_schedule_delete(struct got_worktree *work
 
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-			schedule_for_deletion, &sda, NULL, NULL, 0, 1);
+			schedule_for_deletion, &sda, NULL, NULL, 1, 1);
 		if (err)
 			break;
 	}
@@ -4877,7 +4884,7 @@ got_worktree_revert(struct got_worktree *worktree,
 	rfa.unlink_added_files = 0;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    revert_file, &rfa, NULL, NULL, 0, 0);
+		    revert_file, &rfa, NULL, NULL, 1, 0);
 		if (err)
 			break;
 	}
@@ -5891,7 +5898,7 @@ got_worktree_commit(struct got_object_id **new_commit_
 	cc_arg.allow_bad_symlinks = allow_bad_symlinks;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    collect_commitables, &cc_arg, NULL, NULL, 0, 0);
+		    collect_commitables, &cc_arg, NULL, NULL, 1, 0);
 		if (err)
 			goto done;
 	}
@@ -6830,7 +6837,7 @@ got_worktree_rebase_abort(struct got_worktree *worktre
 	rfa.repo = repo;
 	rfa.unlink_added_files = 0;
 	err = worktree_status(worktree, "", fileindex, repo,
-	    revert_file, &rfa, NULL, NULL, 0, 0);
+	    revert_file, &rfa, NULL, NULL, 1, 0);
 	if (err)
 		goto sync;
 
@@ -7184,7 +7191,7 @@ got_worktree_histedit_abort(struct got_worktree *workt
 	rfa.repo = repo;
 	rfa.unlink_added_files = 0;
 	err = worktree_status(worktree, "", fileindex, repo,
-	    revert_file, &rfa, NULL, NULL, 0, 0);
+	    revert_file, &rfa, NULL, NULL, 1, 0);
 	if (err)
 		goto sync;
 
@@ -7583,7 +7590,7 @@ got_worktree_merge_commit(struct got_object_id **new_c
 	cc_arg.have_staged_files = have_staged_files;
 	cc_arg.allow_bad_symlinks = allow_bad_symlinks;
 	err = worktree_status(worktree, "", fileindex, repo,
-	    collect_commitables, &cc_arg, NULL, NULL, 0, 0);
+	    collect_commitables, &cc_arg, NULL, NULL, 1, 0);
 	if (err)
 		goto done;
 
@@ -7892,7 +7899,7 @@ got_worktree_merge_abort(struct got_worktree *worktree
 	rfa.repo = repo;
 	rfa.unlink_added_files = 1;
 	err = worktree_status(worktree, "", fileindex, repo,
-	    revert_file, &rfa, NULL, NULL, 0, 0);
+	    revert_file, &rfa, NULL, NULL, 1, 0);
 	if (err)
 		goto sync;
 
@@ -8179,7 +8186,7 @@ got_worktree_stage(struct got_worktree *worktree,
 	oka.have_changes = 0;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    check_stage_ok, &oka, NULL, NULL, 0, 0);
+		    check_stage_ok, &oka, NULL, NULL, 1, 0);
 		if (err)
 			goto done;
 	}
@@ -8199,7 +8206,7 @@ got_worktree_stage(struct got_worktree *worktree,
 	spa.allow_bad_symlinks = allow_bad_symlinks;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    stage_path, &spa, NULL, NULL, 0, 0);
+		    stage_path, &spa, NULL, NULL, 1, 0);
 		if (err)
 			goto done;
 	}
@@ -8691,7 +8698,7 @@ got_worktree_unstage(struct got_worktree *worktree,
 	upa.patch_arg = patch_arg;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    unstage_path, &upa, NULL, NULL, 0, 0);
+		    unstage_path, &upa, NULL, NULL, 1, 0);
 		if (err)
 			goto done;
 	}