Re: [PATCHv2 28/36] attr: keep attr stack for each check

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 28, 2016, 21:35 UTC
Message-ID: <xmqqvawcxikq.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161028185502.8789-29-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 7 quoted lines

> Instead of having a global attr stack, attach the stack to each check.
> This allows to use the attr in a multithreaded way.
>
>
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

>  attr.c    | 101 +++++++++++++++++++++++++++++++++++++++-----------------------
>  attr.h    |   4 ++-
>  hashmap.h |   2 ++
>  3 files changed, 69 insertions(+), 38 deletions(-)

This looks surprisingly simple ;-)  I like it.

I briefly wondered if the addition of lock/unlock surrounding git_check_attrs() function belongs to [27/36], but that step is not about making things thread-safe and is primarily to prepare existing users to use an updated API that can be made thread-safe in later steps. This [28/36] is the step to have these---so the addition is not out-of-space at all.

Nicely done.

As this starts to pass a fully populated check object down to the callchain that begins at bootstrap_attr_stack(), it makes it easier to add the per-check optimization to read and keep only the relevant entries from the attribute files later, by passing check also to the read_attr_from_file() function.

The "set-direction" thing is not yet thread-safe, but I am not sure what the best way to go there offhand. It somehow feels unnecessary to allow some thread to be going in the GIT_ATTR_CHECKIN direction while others to be going in the GIT_ATTR_CHECKOUT direction, so we probably can leave it at a lower priority for now.