On Mon, Oct 3, 2016 at 1:49 PM, Junio C Hamano <[email protected]> wrote:
> Stefan Beller <[email protected]> writes:
>
>> I am looking at the tip of jc/attr-more and for a minimum
>> thread safety we'd want to change the call sites to be aware of the
>> threads, i.e. instead of doing
>>
> static struct git_attr_check *check;
>> if (!check)
>> check = git_attr_check_initl("crlf", "ident",
>> "filter", "eol", "text",
>> NULL);
>>
>> We'd rather call
>>
>> struct git_attr_check *check;
>> check = git_attr_check_lookup_or_initl_threadsafe(
>> "crlf", "ident", "filter", "eol", "text", NULL);
>> if (!git_check_attr(path, check)) {
>> ...
>
> I actually am hoping that the "static" can be kept so that we can
> minimize the cost of interning these symbols into struct git_attr.
>
> The initialization would thus become something like:
>
> static struct git_attr_check *check;
> git_attr_check_initl(&check, "crlf", "ident", ..., NULL);
>
> The structure will have an array of git_attr[], once interned will
> be shared and used by everybody. _initl() will need to make sure
> that the "check" pointer is updated atomically so that multiple
> people racing to initialize this variable will not step on each
> others' toes.
I see and that mutex to protect the first argument of git_attr_check_initl
is unrelated to the actual pointer; we can use a global mutex for that,
or rather a static scoped mutex in attr.c, such that all parallel
racing git_attr_check_initl try to acquire that init_lock and only
one succeeds and that one makes sure to first initialize a git_attr_check
completely and then atomically storing the pointer to the &check location.
>
> Then the use site would do something like
>
> const char *result[... some size ...];
... some size ... depends on the git_attr_check->check_nr
and not on the path as I first assumed. So when we still
want to go fast here:
static struct git_attr_check *check;
/* hard code 2 as it has to hold results for crlf and ident only */
const char *results[2];
if (!check)
git_attr_check_initl(&check, "crlf", "ident", NULL);
if (nr != check->check_nr)
ALLOC_GROW(results, check->check_nr, alloc);
git_check_attr(path, check, &result);
// result[i] contains the result for the i-th element of check
// Note: git_attr_check_elem seems to be useless now, as the
// results are not stored in there, we only make use of the `attr` key.
