attrcache: don't re-read attrs during checkout

During checkout, assume that the .gitattributes files aren't
modified during the checkout.  Instead, create an "attribute session"
during checkout.  Assume that attribute data read in the same
checkout "session" hasn't been modified since the checkout started.
(But allow subsequent checkouts to invalidate the cache.)

Further, cache nonexistent git_attr_file data even when .gitattributes
files are not found to prevent re-scanning for nonexistent files.

src/attr.c

@@ -29,6 +29,7 @@ git_attr_t git_attr_value(const char *attr)
 
 static int collect_attr_files(
 	git_repository *repo,
+	git_attr_session *attr_session,
 	uint32_t flags,
 	const char *path,
 	git_vector *files);
@@ -57,7 +58,7 @@ int git_attr_get(
 	if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0)
 		return -1;
 
-	if ((error = collect_attr_files(repo, flags, pathname, &files)) < 0)
+	if ((error = collect_attr_files(repo, NULL, flags, pathname, &files)) < 0)
 		goto cleanup;
 
 	memset(&attr, 0, sizeof(attr));
@@ -90,9 +91,10 @@ typedef struct {
 	git_attr_assignment *found;
 } attr_get_many_info;
 
-int git_attr_get_many(
+int git_attr_get_many_with_session(
 	const char **values,
 	git_repository *repo,
+	git_attr_session *attr_session,
 	uint32_t flags,
 	const char *pathname,
 	size_t num_attr,
@@ -115,7 +117,7 @@ int git_attr_get_many(
 	if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0)
 		return -1;
 
-	if ((error = collect_attr_files(repo, flags, pathname, &files)) < 0)
+	if ((error = collect_attr_files(repo, attr_session, flags, pathname, &files)) < 0)
 		goto cleanup;
 
 	info = git__calloc(num_attr, sizeof(attr_get_many_info));
@@ -161,6 +163,17 @@ cleanup:
 	return error;
 }
 
+int git_attr_get_many(
+	const char **values,
+	git_repository *repo,
+	uint32_t flags,
+	const char *pathname,
+	size_t num_attr,
+	const char **names)
+{
+	return git_attr_get_many_with_session(
+		values, repo, NULL, flags, pathname, num_attr, names);
+}
 
 int git_attr_foreach(
 	git_repository *repo,
@@ -183,7 +196,7 @@ int git_attr_foreach(
 	if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0)
 		return -1;
 
-	if ((error = collect_attr_files(repo, flags, pathname, &files)) < 0 ||
+	if ((error = collect_attr_files(repo, NULL, flags, pathname, &files)) < 0 ||
 		(error = git_strmap_alloc(&seen)) < 0)
 		goto cleanup;
 
@@ -219,6 +232,7 @@ cleanup:
 
 static int preload_attr_file(
 	git_repository *repo,
+	git_attr_session *attr_session,
 	git_attr_file_source source,
 	const char *base,
 	const char *file)
@@ -229,19 +243,22 @@ static int preload_attr_file(
 	if (!file)
 		return 0;
 	if (!(error = git_attr_cache__get(
-			&preload, repo, source, base, file, git_attr_file__parse_buffer)))
+			&preload, repo, attr_session, source, base, file, git_attr_file__parse_buffer)))
 		git_attr_file__free(preload);
 
 	return error;
 }
 
-static int attr_setup(git_repository *repo)
+static int attr_setup(git_repository *repo, git_attr_session *attr_session)
 {
 	int error = 0;
 	const char *workdir = git_repository_workdir(repo);
 	git_index *idx = NULL;
 	git_buf sys = GIT_BUF_INIT;
 
+	if (attr_session && attr_session->setup)
+		return 0;
+
 	if ((error = git_attr_cache__init(repo)) < 0)
 		return error;
 
@@ -251,7 +268,7 @@ static int attr_setup(git_repository *repo)
 
 	if (!(error = git_sysdir_find_system_file(&sys, GIT_ATTR_FILE_SYSTEM))) {
 		error = preload_attr_file(
-			repo, GIT_ATTR_FILE__FROM_FILE, NULL, sys.ptr);
+			repo, attr_session, GIT_ATTR_FILE__FROM_FILE, NULL, sys.ptr);
 		git_buf_free(&sys);
 	}
 	if (error < 0) {
@@ -263,25 +280,28 @@ static int attr_setup(git_repository *repo)
 	}
 
 	if ((error = preload_attr_file(
-			repo, GIT_ATTR_FILE__FROM_FILE,
+			repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
 			NULL, git_repository_attr_cache(repo)->cfg_attr_file)) < 0)
 		return error;
 
 	if ((error = preload_attr_file(
-			repo, GIT_ATTR_FILE__FROM_FILE,
+			repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
 			git_repository_path(repo), GIT_ATTR_FILE_INREPO)) < 0)
 		return error;
 
 	if (workdir != NULL &&
 		(error = preload_attr_file(
-			repo, GIT_ATTR_FILE__FROM_FILE, workdir, GIT_ATTR_FILE)) < 0)
+			repo, attr_session, GIT_ATTR_FILE__FROM_FILE, workdir, GIT_ATTR_FILE)) < 0)
 		return error;
 
 	if ((error = git_repository_index__weakptr(&idx, repo)) < 0 ||
 		(error = preload_attr_file(
-			repo, GIT_ATTR_FILE__FROM_INDEX, NULL, GIT_ATTR_FILE)) < 0)
+			repo, attr_session, GIT_ATTR_FILE__FROM_INDEX, NULL, GIT_ATTR_FILE)) < 0)
 		return error;
 
+	if (attr_session)
+		attr_session->setup = 1;
+
 	return error;
 }
 
@@ -321,6 +341,7 @@ int git_attr_add_macro(
 
 typedef struct {
 	git_repository *repo;
+	git_attr_session *attr_session;
 	uint32_t flags;
 	const char *workdir;
 	git_index *index;
@@ -356,6 +377,7 @@ static int attr_decide_sources(
 
 static int push_attr_file(
 	git_repository *repo,
+	git_attr_session *attr_session,
 	git_vector *list,
 	git_attr_file_source source,
 	const char *base,
@@ -364,8 +386,9 @@ static int push_attr_file(
 	int error = 0;
 	git_attr_file *file = NULL;
 
-	error = git_attr_cache__get(
-		&file, repo, source, base, filename, git_attr_file__parse_buffer);
+	error = git_attr_cache__get(&file, repo, attr_session,
+		source, base, filename, git_attr_file__parse_buffer);
+
 	if (error < 0)
 		return error;
 
@@ -387,8 +410,8 @@ static int push_one_attr(void *ref, const char *path)
 		info->flags, info->workdir != NULL, info->index != NULL, src);
 
 	for (i = 0; !error && i < n_src; ++i)
-		error = push_attr_file(
-			info->repo, info->files, src[i], path, GIT_ATTR_FILE);
+		error = push_attr_file(info->repo, info->attr_session,
+			info->files, src[i], path, GIT_ATTR_FILE);
 
 	return error;
 }
@@ -407,6 +430,7 @@ static void release_attr_files(git_vector *files)
 
 static int collect_attr_files(
 	git_repository *repo,
+	git_attr_session *attr_session,
 	uint32_t flags,
 	const char *path,
 	git_vector *files)
@@ -416,7 +440,7 @@ static int collect_attr_files(
 	const char *workdir = git_repository_workdir(repo);
 	attr_walk_up_info info = { NULL };
 
-	if ((error = attr_setup(repo)) < 0)
+	if ((error = attr_setup(repo, attr_session)) < 0)
 		return error;
 
 	/* Resolve path in a non-bare repo */
@@ -435,12 +459,13 @@ static int collect_attr_files(
 	 */
 
 	error = push_attr_file(
-		repo, files, GIT_ATTR_FILE__FROM_FILE,
+		repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
 		git_repository_path(repo), GIT_ATTR_FILE_INREPO);
 	if (error < 0)
 		goto cleanup;
 
-	info.repo  = repo;
+	info.repo = repo;
+	info.attr_session = attr_session;
 	info.flags = flags;
 	info.workdir = workdir;
 	if (git_repository_index__weakptr(&info.index, repo) < 0)
@@ -457,7 +482,7 @@ static int collect_attr_files(
 
 	if (git_repository_attr_cache(repo)->cfg_attr_file != NULL) {
 		error = push_attr_file(
-			repo, files, GIT_ATTR_FILE__FROM_FILE,
+			repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
 			NULL, git_repository_attr_cache(repo)->cfg_attr_file);
 		if (error < 0)
 			goto cleanup;
@@ -467,7 +492,8 @@ static int collect_attr_files(
 		error = git_sysdir_find_system_file(&dir, GIT_ATTR_FILE_SYSTEM);
 		if (!error)
 			error = push_attr_file(
-				repo, files, GIT_ATTR_FILE__FROM_FILE, NULL, dir.ptr);
+				repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
+				NULL, dir.ptr);
 		else if (error == GIT_ENOTFOUND) {
 			giterr_clear();
 			error = 0;

src/attr_file.c

@@ -96,6 +96,7 @@ static int attr_file_oid_from_index(
 int git_attr_file__load(
 	git_attr_file **out,
 	git_repository *repo,
+	git_attr_session *attr_session,
 	git_attr_file_entry *entry,
 	git_attr_file_source source,
 	git_attr_file_parser parser)
@@ -105,6 +106,7 @@ int git_attr_file__load(
 	git_buf content = GIT_BUF_INIT;
 	git_attr_file *file;
 	struct stat st;
+	bool nonexistent = false;
 
 	*out = NULL;
 
@@ -127,22 +129,16 @@ int git_attr_file__load(
 	case GIT_ATTR_FILE__FROM_FILE: {
 		int fd;
 
-		if (p_stat(entry->fullpath, &st) < 0)
-			return git_path_set_error(errno, entry->fullpath, "stat");
-		if (S_ISDIR(st.st_mode))
-			return GIT_ENOTFOUND;
-
-		/* For open or read errors, return ENOTFOUND to skip item */
+		/* For open or read errors, pretend that we got ENOTFOUND. */
 		/* TODO: issue warning when warning API is available */
 
-		if ((fd = git_futils_open_ro(entry->fullpath)) < 0)
-			return GIT_ENOTFOUND;
-
-		error = git_futils_readbuffer_fd(&content, fd, (size_t)st.st_size);
-		p_close(fd);
-
-		if (error < 0)
-			return GIT_ENOTFOUND;
+		if (p_stat(entry->fullpath, &st) < 0 ||
+			S_ISDIR(st.st_mode) ||
+			(fd = git_futils_open_ro(entry->fullpath)) < 0 ||
+			(error = git_futils_readbuffer_fd(&content, fd, (size_t)st.st_size)) < 0)
+			nonexistent = true;
+		else
+			p_close(fd);
 
 		break;
 	}
@@ -154,13 +150,21 @@ int git_attr_file__load(
 	if ((error = git_attr_file__new(&file, entry, source)) < 0)
 		goto cleanup;
 
+	/* store the key of the attr_reader; don't bother with cache
+	 * invalidation during the same attr reader session.
+	 */
+	if (attr_session)
+		file->session_key = attr_session->key;
+
 	if (parser && (error = parser(repo, file, git_buf_cstr(&content))) < 0) {
 		git_attr_file__free(file);
 		goto cleanup;
 	}
 
-	/* write cache breaker */
-	if (source == GIT_ATTR_FILE__FROM_INDEX)
+	/* write cache breakers */
+	if (nonexistent)
+		file->nonexistent = 1;
+	else if (source == GIT_ATTR_FILE__FROM_INDEX)
 		git_oid_cpy(&file->cache_data.oid, git_blob_id(blob));
 	else if (source == GIT_ATTR_FILE__FROM_FILE)
 		git_futils_filestamp_set_from_stat(&file->cache_data.stamp, &st);
@@ -175,11 +179,22 @@ cleanup:
 	return error;
 }
 
-int git_attr_file__out_of_date(git_repository *repo, git_attr_file *file)
+int git_attr_file__out_of_date(
+	git_repository *repo,
+	git_attr_session *attr_session,
+	git_attr_file *file)
 {
 	if (!file)
 		return 1;
 
+	/* we are never out of date if we just created this data in the same
+	 * attr_session; otherwise, nonexistent files must be invalidated
+	 */
+	if (attr_session && attr_session->key == file->session_key)
+		return 0;
+	else if (file->nonexistent)
+		return 1;
+
 	switch (file->source) {
 	case GIT_ATTR_FILE__IN_MEMORY:
 		return 0;
@@ -831,3 +846,11 @@ void git_attr_rule__free(git_attr_rule *rule)
 	git__free(rule);
 }
 
+int git_attr_session__init(git_attr_session *session, git_repository *repo)
+{
+	assert(repo);
+
+	session->key = git_atomic_inc(&repo->attr_session_key);
+
+	return 0;
+}

src/attr_file.h

@@ -38,11 +38,11 @@
 	 GIT_ATTR_FNMATCH_ALLOWMACRO | GIT_ATTR_FNMATCH_NOLEADINGDIR)
 
 typedef enum {
-	GIT_ATTR_FILE__IN_MEMORY  = 0,
-	GIT_ATTR_FILE__FROM_FILE  = 1,
-	GIT_ATTR_FILE__FROM_INDEX = 2,
+	GIT_ATTR_FILE__IN_MEMORY   = 0,
+	GIT_ATTR_FILE__FROM_FILE   = 1,
+	GIT_ATTR_FILE__FROM_INDEX  = 2,
 
-	GIT_ATTR_FILE_NUM_SOURCES = 3
+	GIT_ATTR_FILE_NUM_SOURCES  = 3
 } git_attr_file_source;
 
 extern const char *git_attr__true;
@@ -84,6 +84,8 @@ typedef struct {
 	git_attr_file_source source;
 	git_vector rules;			/* vector of <rule*> or <fnmatch*> */
 	git_pool pool;
+	unsigned int nonexistent:1;
+	int session_key;
 	union {
 		git_oid oid;
 		git_futils_filestamp stamp;
@@ -96,11 +98,6 @@ struct git_attr_file_entry {
 	char fullpath[GIT_FLEX_ARRAY];
 };
 
-typedef int (*git_attr_file_parser)(
-	git_repository *repo,
-	git_attr_file *file,
-	const char *data);
-
 typedef struct {
 	git_buf  full;
 	char    *path;
@@ -108,6 +105,32 @@ typedef struct {
 	int      is_dir;
 } git_attr_path;
 
+/* A git_attr_session can provide an "instance" of reading, to prevent cache
+ * invalidation during a single operation instance (like checkout).
+ */
+
+typedef struct {
+	int key;
+	unsigned int setup;
+} git_attr_session;
+
+extern int git_attr_session__init(git_attr_session *attr_session, git_repository *repo);
+extern void git_attr_session__free(git_attr_session *session);
+
+extern int git_attr_get_many_with_session(
+	const char **values_out,
+	git_repository *repo,
+	git_attr_session *attr_session,
+	uint32_t flags,
+	const char *path,
+	size_t num_attr,
+	const char **names);
+
+typedef int (*git_attr_file_parser)(
+	git_repository *repo,
+	git_attr_file *file,
+	const char *data);
+
 /*
  * git_attr_file API
  */
@@ -122,6 +145,7 @@ void git_attr_file__free(git_attr_file *file);
 int git_attr_file__load(
 	git_attr_file **out,
 	git_repository *repo,
+	git_attr_session *attr_session,
 	git_attr_file_entry *ce,
 	git_attr_file_source source,
 	git_attr_file_parser parser);
@@ -130,7 +154,7 @@ int git_attr_file__load_standalone(
 	git_attr_file **out, const char *path);
 
 int git_attr_file__out_of_date(
-	git_repository *repo, git_attr_file *file);
+	git_repository *repo, git_attr_session *session, git_attr_file *file);
 
 int git_attr_file__parse_buffer(
 	git_repository *repo, git_attr_file *attrs, const char *data);

src/attrcache.c

@@ -196,6 +196,7 @@ cleanup:
 int git_attr_cache__get(
 	git_attr_file **out,
 	git_repository *repo,
+	git_attr_session *attr_session,
 	git_attr_file_source source,
 	const char *base,
 	const char *filename,
@@ -211,8 +212,8 @@ int git_attr_cache__get(
 		return error;
 
 	/* load file if we don't have one or if existing one is out of date */
-	if (!file || (error = git_attr_file__out_of_date(repo, file)) > 0)
-		error = git_attr_file__load(&updated, repo, entry, source, parser);
+	if (!file || (error = git_attr_file__out_of_date(repo, attr_session, file)) > 0)
+		error = git_attr_file__load(&updated, repo, attr_session, entry, source, parser);
 
 	/* if we loaded the file, insert into and/or update cache */
 	if (updated) {

src/attrcache.h

@@ -31,6 +31,7 @@ extern int git_attr_cache__do_init(git_repository *repo);
 extern int git_attr_cache__get(
 	git_attr_file **file,
 	git_repository *repo,
+	git_attr_session *attr_session,
 	git_attr_file_source source,
 	const char *base,
 	const char *filename,

src/checkout.c

@@ -28,6 +28,7 @@
 #include "buf_text.h"
 #include "merge_file.h"
 #include "path.h"
+#include "attr.h"
 
 /* See docs/checkout-internals.md for more information */
 
@@ -69,6 +70,7 @@ typedef struct {
 	size_t completed_steps;
 	git_checkout_perfdata perfdata;
 	git_buf last_mkdir;
+	git_attr_session attr_session;
 } checkout_data;
 
 typedef struct {
@@ -1425,8 +1427,8 @@ static int blob_content_to_file(
 		hint_path = path;
 
 	if (!data->opts.disable_filters)
-		error = git_filter_list_load(
-			&fl, git_blob_owner(blob), blob, hint_path,
+		error = git_filter_list__load_with_attr_session(
+			&fl, data->repo, &data->attr_session, blob, hint_path,
 			GIT_FILTER_TO_WORKTREE, GIT_FILTER_OPT_DEFAULT);
 
 	if (!error)
@@ -2008,7 +2010,8 @@ static int checkout_write_merge(
 		in_data.ptr = (char *)result.ptr;
 		in_data.size = result.len;
 
-		if ((error = git_filter_list_load(&fl, data->repo, NULL, git_buf_cstr(&path_workdir),
+		if ((error = git_filter_list__load_with_attr_session(
+				&fl, data->repo, &data->attr_session, NULL, git_buf_cstr(&path_workdir),
 				GIT_FILTER_TO_WORKTREE, GIT_FILTER_OPT_DEFAULT)) < 0 ||
 			(error = git_filter_list_apply_to_data(&out_data, fl, &in_data)) < 0)
 			goto done;
@@ -2218,6 +2221,8 @@ static void checkout_data_clear(checkout_data *data)
 
 	git_index_free(data->index);
 	data->index = NULL;
+
+	git_attr_session__free(&data->attr_session);
 }
 
 static int checkout_data_init(
@@ -2360,6 +2365,8 @@ static int checkout_data_init(
 
 	data->workdir_len = git_buf_len(&data->path);
 
+	git_attr_session__init(&data->attr_session, data->repo);
+
 cleanup:
 	if (error < 0)
 		checkout_data_clear(data);

src/filter.c

@@ -394,15 +394,19 @@ static int filter_list_new(
 }
 
 static int filter_list_check_attributes(
-	const char ***out, git_filter_def *fdef, const git_filter_source *src)
+	const char ***out,
+	git_repository *repo,
+	git_attr_session *attr_session,
+	git_filter_def *fdef,
+	const git_filter_source *src)
 {
 	int error;
 	size_t i;
 	const char **strs = git__calloc(fdef->nattrs, sizeof(const char *));
 	GITERR_CHECK_ALLOC(strs);
 
-	error = git_attr_get_many(
-		strs, src->repo, 0, src->path, fdef->nattrs, fdef->attrs);
+	error = git_attr_get_many_with_session(
+		strs, repo, attr_session, 0, src->path, fdef->nattrs, fdef->attrs);
 
 	/* if no values were found but no matches are needed, it's okay! */
 	if (error == GIT_ENOTFOUND && !fdef->nmatches) {
@@ -448,9 +452,10 @@ int git_filter_list_new(
 	return filter_list_new(out, &src);
 }
 
-int git_filter_list_load(
+int git_filter_list__load_with_attr_session(
 	git_filter_list **filters,
 	git_repository *repo,
+	git_attr_session *attr_session,
 	git_blob *blob, /* can be NULL */
 	const char *path,
 	git_filter_mode_t mode,
@@ -481,7 +486,9 @@ int git_filter_list_load(
 			continue;
 
 		if (fdef->nattrs > 0) {
-			error = filter_list_check_attributes(&values, fdef, &src);
+			error = filter_list_check_attributes(
+				&values, repo, attr_session, fdef, &src);
+
 			if (error == GIT_ENOTFOUND) {
 				error = 0;
 				continue;
@@ -523,6 +530,18 @@ int git_filter_list_load(
 	return error;
 }
 
+int git_filter_list_load(
+	git_filter_list **filters,
+	git_repository *repo,
+	git_blob *blob, /* can be NULL */
+	const char *path,
+	git_filter_mode_t mode,
+	uint32_t options)
+{
+	return git_filter_list__load_with_attr_session(
+		filters, repo, NULL, blob, path, mode, options);
+}
+
 void git_filter_list_free(git_filter_list *fl)
 {
 	uint32_t i;

src/filter.h

@@ -8,6 +8,7 @@
 #define INCLUDE_filter_h__
 
 #include "common.h"
+#include "attr_file.h"
 #include "git2/filter.h"
 
 /* Amount of file to examine for NUL byte when checking binary-ness */
@@ -25,6 +26,15 @@ typedef enum {
 
 extern void git_filter_free(git_filter *filter);
 
+extern int git_filter_list__load_with_attr_session(
+	git_filter_list **filters,
+	git_repository *repo,
+	git_attr_session *attr_session,
+	git_blob *blob, /* can be NULL */
+	const char *path,
+	git_filter_mode_t mode,
+	uint32_t options);
+
 /*
  * Available filters
  */

src/ignore.c

@@ -161,7 +161,7 @@ static int push_ignore_file(
 	git_attr_file *file = NULL;
 
 	error = git_attr_cache__get(
-		&file, ignores->repo, GIT_ATTR_FILE__FROM_FILE,
+		&file, ignores->repo, NULL, GIT_ATTR_FILE__FROM_FILE,
 		base, filename, parse_ignore_file);
 	if (error < 0)
 		return error;
@@ -189,7 +189,7 @@ static int get_internal_ignores(git_attr_file **out, git_repository *repo)
 		return error;
 
 	error = git_attr_cache__get(
-		out, repo, GIT_ATTR_FILE__IN_MEMORY, NULL, GIT_IGNORE_INTERNAL, NULL);
+		out, repo, NULL, GIT_ATTR_FILE__IN_MEMORY, NULL, GIT_IGNORE_INTERNAL, NULL);
 
 	/* if internal rules list is empty, insert default rules */
 	if (!error && !(*out)->rules.length)

src/repository.h

@@ -133,6 +133,8 @@ struct git_repository {
 		has_8dot3_default:1;
 	unsigned int lru_counter;
 
+	git_atomic attr_session_key;
+
 	git_cvar_value cvar_cache[GIT_CVAR_CACHE_MAX];
 };