Re: [PATCH 2/2] wildmatch: use the new precompiling wildmatch()

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Feb 26 2018, Duy Nguyen jotted:

> On Mon, Feb 26, 2018 at 3:35 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> diff --git a/config.c b/config.c
>> index b0c20e6cb8..0f595de971 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -210,6 +210,7 @@ static int include_by_gitdir(const struct config_options *opts,
>>         int ret = 0, prefix;
>>         const char *git_dir;
>>         int already_tried_absolute = 0;
>> +       struct wildmatch_compiled *wildmatch_compiled = NULL;
>>
>>         if (opts->git_dir)
>>                 git_dir = opts->git_dir;
>> @@ -237,8 +238,10 @@ static int include_by_gitdir(const struct config_options *opts,
>>                         goto done;
>>         }
>>
>> -       ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,
>> -                        icase ? WM_CASEFOLD : 0);
>> +       if (!wildmatch_compiled)
>> +               wildmatch_compiled = wildmatch_compile(pattern.buf + prefix,
>> +                                                      icase ? WM_CASEFOLD : 0);
>> +       ret = !wildmatch_match(wildmatch_compiled, text.buf + prefix);
>
> This is a one-shot matching.

It will match once *or* twice depending on how the goto in that codepath
does.

> Is it worth converting to the new interface?

I moved it more as a showcase, to show how the interface looks like in
common scenarios. It obviously won't be a performance bottleneck or
anything like that for this specific codepath.

>>
>>         if (!ret && !already_tried_absolute) {
>>                 /*
>> @@ -257,6 +260,7 @@ static int include_by_gitdir(const struct config_options *opts,
>>  done:
>>         strbuf_release(&pattern);
>>         strbuf_release(&text);
>> +       wildmatch_free(wildmatch_compiled);
>>         return ret;
>>  }
>>
>> diff --git a/refs.c b/refs.c
>> index 20ba82b434..c631793d1e 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -213,7 +213,7 @@ char *resolve_refdup(const char *refname, int resolve_flags,
>>
>>  /* The argument to filter_refs */
>>  struct ref_filter {
>> -       const char *pattern;
>> +       struct wildmatch_compiled *code;
>
> This actually makes me think if we should name this struct wildmatch_pattern.

Yeah, that's a better name.

>>         each_ref_fn *fn;
>>         void *cb_data;
>>  };
>> @@ -291,7 +291,7 @@ static int filter_refs(const char *refname, const struct object_id *oid,
>>  {
>>         struct ref_filter *filter = (struct ref_filter *)data;
>>
>> -       if (wildmatch(filter->pattern, refname, 0))
>> +       if (wildmatch_match(filter->code, refname))
>>                 return 0;
>>         return filter->fn(refname, oid, flags, filter->cb_data);
>>  }
>> @@ -454,12 +454,13 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
>>                 strbuf_addch(&real_pattern, '*');
>>         }
>>
>> -       filter.pattern = real_pattern.buf;
>> +       filter.code = wildmatch_compile(real_pattern.buf, 0);
>>         filter.fn = fn;
>>         filter.cb_data = cb_data;
>>         ret = for_each_ref(filter_refs, &filter);
>>
>>         strbuf_release(&real_pattern);
>> +       wildmatch_free(filter.code);
>>         return ret;
>>  }
>>
>> --
>> 2.15.1.424.g9478a66081
>>