From: Junio C Hamano <gitster@pobox•com>
To: Duy Nguyen <pclouds@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH/RFC 4/4] attr: avoid heavy work when we know the specified attr is not defined
Date: Mon, 15 Dec 2014 09:30:35 -0800 [thread overview]
Message-ID: <xmqqfvcges78.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20141215005018.GA31006@lanh> (Duy Nguyen's message of "Mon, 15 Dec 2014 07:50:18 +0700")
Duy Nguyen <pclouds@gmail•com> writes:
> On Tue, Dec 09, 2014 at 04:18:57PM -0800, Junio C Hamano wrote:
> ...
>> I smell that a much better split is possible.
>> ...
>
> Something like this? Definitely looks better.
Yeah, I was lazy and did not try it myself to see what the end
result would look like when I commented, but doing it this way
avoids needless repetitions.
The comment block before the collect_*_attrs function need to be
adjusted to match the updated behaviour, though.
Thanks.
> -- 8< --
> diff --git a/attr.c b/attr.c
> index b80e52b..0f828e3 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -33,9 +33,11 @@ struct git_attr {
> unsigned h;
> int attr_nr;
> int maybe_macro;
> + int maybe_real;
> char name[FLEX_ARRAY];
> };
> static int attr_nr;
> +static int cannot_trust_maybe_real;
>
> static struct git_attr_check *check_all_attr;
> static struct git_attr *(git_attr_hash[HASHSIZE]);
> @@ -97,6 +99,7 @@ static struct git_attr *git_attr_internal(const char *name, int len)
> a->next = git_attr_hash[pos];
> a->attr_nr = attr_nr++;
> a->maybe_macro = 0;
> + a->maybe_real = 0;
> git_attr_hash[pos] = a;
>
> REALLOC_ARRAY(check_all_attr, attr_nr);
> @@ -269,6 +272,10 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
> /* Second pass to fill the attr_states */
> for (cp = states, i = 0; *cp; i++) {
> cp = parse_attr(src, lineno, cp, &(res->state[i]));
> + if (!is_macro)
> + res->state[i].attr->maybe_real = 1;
> + if (res->state[i].attr->maybe_macro)
> + cannot_trust_maybe_real = 1;
> }
>
> return res;
> @@ -713,7 +720,9 @@ static int macroexpand_one(int nr, int rem)
> * Collect all attributes for path into the array pointed to by
> * check_all_attr.
> */
> -static void collect_all_attrs(const char *path)
> +static void collect_some_attrs(const char *path, int num,
> + struct git_attr_check *check)
> +
> {
> struct attr_stack *stk;
> int i, pathlen, rem, dirlen;
> @@ -736,6 +745,19 @@ static void collect_all_attrs(const char *path)
> 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) {
> + rem = 0;
> + for (i = 0; i < num; i++) {
> + if (!check[i].attr->maybe_real) {
> + struct git_attr_check *c;
> + c = check_all_attr + check[i].attr->attr_nr;
> + c->value = ATTR__UNSET;
> + rem++;
> + }
> + }
> + if (rem == num)
> + return;
> + }
>
> rem = attr_nr;
> for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
> @@ -746,7 +768,7 @@ int git_check_attr(const char *path, int num, struct git_attr_check *check)
> {
> int i;
>
> - collect_all_attrs(path);
> + collect_some_attrs(path, num, check);
>
> for (i = 0; i < num; i++) {
> const char *value = check_all_attr[check[i].attr->attr_nr].value;
> @@ -762,7 +784,7 @@ int git_all_attrs(const char *path, int *num, struct git_attr_check **check)
> {
> int i, count, j;
>
> - collect_all_attrs(path);
> + collect_some_attrs(path, 0, NULL);
>
> /* Count the number of attributes that are set. */
> count = 0;
> -- 8< --
next prev parent reply other threads:[~2014-12-15 17:30 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-09 13:53 [PATCH/RFC 0/4] some attr optimizations Nguyễn Thái Ngọc Duy
2014-12-09 13:53 ` [PATCH 1/4] attr.c: rename global var attr_nr to git_attr_nr Nguyễn Thái Ngọc Duy
2014-12-09 23:54 ` Junio C Hamano
2014-12-09 13:53 ` [PATCH 2/4] attr.c: split path processing code out of collect_all_attrs() Nguyễn Thái Ngọc Duy
2014-12-09 13:53 ` [PATCH/RFC 3/4] attr: do not attempt to expand when we know it's not a macro Nguyễn Thái Ngọc Duy
2014-12-09 23:27 ` Eric Sunshine
2014-12-09 23:56 ` Junio C Hamano
2014-12-09 13:53 ` [PATCH/RFC 4/4] attr: avoid heavy work when we know the specified attr is not defined Nguyễn Thái Ngọc Duy
2014-12-10 0:18 ` Junio C Hamano
2014-12-15 0:50 ` Duy Nguyen
2014-12-15 17:30 ` Junio C Hamano [this message]
2014-12-27 23:39 ` [PATCH v2 0/3] some attr optimizations Nguyễn Thái Ngọc Duy
2014-12-27 23:39 ` [PATCH v2 1/3] attr.c: rename arg name attr_nr to avoid shadowing the global one Nguyễn Thái Ngọc Duy
2014-12-27 23:39 ` [PATCH v2 2/3] attr: do not attempt to expand when we know it's not a macro Nguyễn Thái Ngọc Duy
2014-12-27 23:59 ` Eric Sunshine
2014-12-27 23:39 ` [PATCH v2 3/3] attr: avoid heavy work when we know the specified attr is not defined Nguyễn Thái Ngọc Duy
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqfvcges78.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox