Stefan Beller [off-list ref] 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.
Then the use site would do something like
const char *result[... some size ...];
git_check_attr(path, check, result[]);
to receive result regardless of anybody else who is using the same
"check" structure to ask the same question (for different paths).
On Mon, Oct 3, 2016 at 1:49 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
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;
quoted
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.