git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com, bmwill@google.com, pclouds@gmail.com
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: [PATCH 21/35] attr.c: correct ugly hack for git_all_attrs()
Date: Thu, 10 Nov 2016 12:34:14 -0800	[thread overview]
Message-ID: <20161110203428.30512-22-sbeller@google.com> (raw)
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>

From: Junio C Hamano <gitster@pobox.com>

The collect_some_attrs() function has an ugly hack since

06a604e6 (attr: avoid heavy work when we know the specified attr is
not defined, 2014-12-28) added an optimization that relies on the
fact that the caller knows what attributes it is interested in, so
that we can leave once we know the final answer for all the
attributes the caller asked.

git_all_attrs() that asks "what attributes are on this path?"
however does not know what attributes it is interested in, other
than the vague "we are interested in all of them", which is not a
very useful thing to say.  As a way to disable this optimization
for this caller, the said commit added a code to skip it when
the caller passes a NULL for the check structure.

However, it skipped the optimization not when check is NULL, but
when the number of attributes being checked is 0, which is
unnecessarily pessimistic.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 attr.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/attr.c b/attr.c
index e18098c..48460d2 100644
--- a/attr.c
+++ b/attr.c
@@ -748,8 +748,8 @@ static int attr_check_is_dynamic(const struct git_attr_check *check)
 
 /*
  * Collect attributes for path into the array pointed to by
- * check_all_attr. If num is non-zero, only attributes in check[] are
- * collected. Otherwise all attributes are collected.
+ * check_all_attr.  If check is not NULL, only attributes in
+ * check[] are collected. Otherwise all attributes are collected.
  */
 static void collect_some_attrs(const char *path, int pathlen,
 			       struct git_attr_check *check)
@@ -759,17 +759,6 @@ static void collect_some_attrs(const char *path, int pathlen,
 	int i, rem, dirlen;
 	const char *cp, *last_slash = NULL;
 	int basename_offset;
-	int num;
-	struct git_attr_check_elem *celem;
-
-	if (!check) {
-		/* Yuck - ugly git_all_attrs() hack! */
-		celem = NULL;
-		num = 0;
-	} else {
-		celem = check->check;
-		num = check->check_nr;
-	}
 
 	for (cp = path; cp < path + pathlen; cp++) {
 		if (*cp == '/' && cp[1])
@@ -786,9 +775,12 @@ static void collect_some_attrs(const char *path, int pathlen,
 	prepare_attr_stack(path, dirlen);
 	for (i = 0; i < attr_nr; i++)
 		check_all_attr[i].value = ATTR__UNKNOWN;
-	if (num && !cannot_trust_maybe_real) {
+
+	if (check && !cannot_trust_maybe_real) {
+		struct git_attr_check_elem *celem = check->check;
+
 		rem = 0;
-		for (i = 0; i < num; i++) {
+		for (i = 0; i < check->check_nr; i++) {
 			if (!celem[i].attr->maybe_real) {
 				struct git_attr_check_elem *c;
 				c = check_all_attr + celem[i].attr->attr_nr;
@@ -796,7 +788,7 @@ static void collect_some_attrs(const char *path, int pathlen,
 				rem++;
 			}
 		}
-		if (rem == num)
+		if (rem == check->check_nr)
 			return;
 	}
 
-- 
2.10.1.469.g00a8914


  parent reply	other threads:[~2016-11-10 20:35 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-10 20:33 [PATCHv3 00/35] Revamp the attr subsystem! Stefan Beller
2016-11-10 20:33 ` [PATCH 01/35] commit.c: use strchrnul() to scan for one line Stefan Beller
2016-11-10 20:33 ` [PATCH 02/35] attr.c: " Stefan Beller
2016-11-10 20:33 ` [PATCH 03/35] attr.c: update a stale comment on "struct match_attr" Stefan Beller
2016-11-10 20:33 ` [PATCH 04/35] attr.c: explain the lack of attr-name syntax check in parse_attr() Stefan Beller
2016-11-10 20:33 ` [PATCH 05/35] attr.c: complete a sentence in a comment Stefan Beller
2016-11-10 20:33 ` [PATCH 06/35] attr.c: mark where #if DEBUG ends more clearly Stefan Beller
2016-11-10 20:34 ` [PATCH 07/35] attr.c: simplify macroexpand_one() Stefan Beller
2016-11-10 20:34 ` [PATCH 08/35] attr.c: tighten constness around "git_attr" structure Stefan Beller
2016-11-10 20:34 ` [PATCH 09/35] attr.c: plug small leak in parse_attr_line() Stefan Beller
2016-11-10 20:34 ` [PATCH 10/35] attr: rename function and struct related to checking attributes Stefan Beller
2016-11-10 20:34 ` [PATCH 11/35] attr: (re)introduce git_check_attr() and struct git_attr_check Stefan Beller
2016-11-10 20:34 ` [PATCH 12/35] attr: convert git_all_attrs() to use "struct git_attr_check" Stefan Beller
2016-11-10 20:34 ` [PATCH 13/35] attr: convert git_check_attrs() callers to use the new API Stefan Beller
2016-11-10 20:34 ` [PATCH 14/35] attr: retire git_check_attrs() API Stefan Beller
2016-11-10 20:34 ` [PATCH 15/35] attr: add counted string version of git_check_attr() Stefan Beller
2016-11-10 20:34 ` [PATCH 16/35] attr: expose validity check for attribute names Stefan Beller
2016-11-10 20:34 ` [PATCH 17/35] attr: support quoting pathname patterns in C style Stefan Beller
2016-11-10 20:34 ` [PATCH 18/35] attr.c: add push_stack() helper Stefan Beller
2016-11-10 20:34 ` [PATCH 19/35] attr.c: pass struct git_attr_check down the callchain Stefan Beller
2016-11-10 20:34 ` [PATCH 20/35] attr.c: rename a local variable check Stefan Beller
2016-11-10 20:34 ` Stefan Beller [this message]
2016-11-10 20:34 ` [PATCH 22/35] attr.c: introduce empty_attr_check_elems() Stefan Beller
2016-11-10 20:34 ` [PATCH 23/35] attr.c: always pass check[] to collect_some_attrs() Stefan Beller
2016-11-10 20:34 ` [PATCH 24/35] attr.c: outline the future plans by heavily commenting Stefan Beller
2016-11-10 20:34 ` [PATCH 25/35] attr: make git_check_attr_counted static Stefan Beller
2016-11-10 20:34 ` [PATCH 26/35] attr: convert to new threadsafe API Stefan Beller
2016-11-10 20:34 ` [PATCH 27/35] attr: keep attr stack for each check Stefan Beller
2016-11-10 20:34 ` [PATCH 28/35] Documentation: fix a typo Stefan Beller
2016-11-10 20:34 ` [PATCH 29/35] pathspec: move long magic parsing out of prefix_pathspec Stefan Beller
2016-11-10 20:34 ` [PATCH 30/35] pathspec: move prefix check out of the inner loop Stefan Beller
2016-11-10 20:34 ` [PATCH 31/35] pathspec: allow querying for attributes Stefan Beller
2016-11-22 10:41   ` Duy Nguyen
2016-11-22 17:26     ` Stefan Beller
2016-11-23  9:38       ` Duy Nguyen
2016-11-28 18:02   ` Brandon Williams
2016-11-28 18:03   ` Brandon Williams
2016-11-28 22:11   ` Brandon Williams
2016-11-29 17:37     ` Stefan Beller
2016-11-10 20:34 ` [PATCH 32/35] pathspec: allow escaped query values Stefan Beller
2016-11-10 20:34 ` [PATCH 33/35] submodule update: add `--init-default-path` switch Stefan Beller
2016-11-10 20:34 ` [PATCH 34/35] clone: add --init-submodule=<pathspec> switch Stefan Beller
2016-11-10 20:34 ` [PATCH 35/35] completion: clone can initialize specific submodules Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161110203428.30512-22-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).