@matheusbsbs / matheus-bagliano / commits / dc81cf377c

attr: store attribute stack in attr_check structure

The last big hurdle towards a thread-safe API for the attribute system is the reliance on a global attribute stack that is modified during each call into the attribute system. This patch removes this global stack and instead a stack is stored locally in each attr_check instance. This opens up the opportunity for future optimizations to customize the attribute stack for the attributes that a particular attr_check struct is interested in. One caveat with pushing the attribute stack into the attr_check structure is that the attribute system now needs to keep track of all active attr_check instances. Due to the direction mechanism the stack needs to be dropped when the direction is switched. In order to ensure correctness when the direction is changed the attribute system needs to iterate through all active attr_check instances and drop each of their stacks. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Williams committed Jan 27, 2017 at 18:02 UTC dc81cf377cd25193a9cf044767917e4f5553c285

2 files changed +199 -89

attr.c

+196 -88

@@ -445,17 +445,16 @@ fail_return:

445 * .gitignore file and info/excludes file as a fallback.

446 */

447

448 -/* NEEDSWORK: This will become per git_attr_check */

449 -static struct attr_stack {

448 +struct attr_stack {

449 struct attr_stack *prev;

450 char *origin;

451 size_t originlen;

452 unsigned num_matches;

453 unsigned alloc;

454 struct match_attr **attrs;

456 -} *attr_stack;

455 +};

456

458 -static void free_attr_elem(struct attr_stack *e)

457 +static void attr_stack_free(struct attr_stack *e)

458 {

459 int i;

460 free(e->origin);

@@ -478,9 +477,96 @@ static void free_attr_elem(struct attr_stack *e)

477 free(e);

478 }

479

480 +static void drop_attr_stack(struct attr_stack **stack)

481 +{

482 + while (*stack) {

483 + struct attr_stack *elem = *stack;

484 + *stack = elem->prev;

485 + attr_stack_free(elem);

486 + }

487 +}

488 +

489 +/* List of all attr_check structs; access should be surrounded by mutex */

490 +static struct check_vector {

491 + size_t nr;

492 + size_t alloc;

493 + struct attr_check **checks;

494 +#ifndef NO_PTHREADS

495 + pthread_mutex_t mutex;

496 +#endif

497 +} check_vector;

498 +

499 +static inline void vector_lock(void)

500 +{

501 +#ifndef NO_PTHREADS

502 + pthread_mutex_lock(&check_vector.mutex);

503 +#endif

504 +}

505 +

506 +static inline void vector_unlock(void)

507 +{

508 +#ifndef NO_PTHREADS

509 + pthread_mutex_unlock(&check_vector.mutex);

510 +#endif

511 +}

512 +

513 +static void check_vector_add(struct attr_check *c)

514 +{

515 + vector_lock();

516 +

517 + ALLOC_GROW(check_vector.checks,

518 + check_vector.nr + 1,

519 + check_vector.alloc);

520 + check_vector.checks[check_vector.nr++] = c;

521 +

522 + vector_unlock();

523 +}

524 +

525 +static void check_vector_remove(struct attr_check *check)

526 +{

527 + int i;

528 +

529 + vector_lock();

530 +

531 + /* Find entry */

532 + for (i = 0; i < check_vector.nr; i++)

533 + if (check_vector.checks[i] == check)

534 + break;

535 +

536 + if (i >= check_vector.nr)

537 + die("BUG: no entry found");

538 +

539 + /* shift entries over */

540 + for (; i < check_vector.nr - 1; i++)

541 + check_vector.checks[i] = check_vector.checks[i + 1];

542 +

543 + check_vector.nr--;

544 +

545 + vector_unlock();

546 +}

547 +

548 +/* Iterate through all attr_check instances and drop their stacks */

549 +static void drop_all_attr_stacks(void)

550 +{

551 + int i;

552 +

553 + vector_lock();

554 +

555 + for (i = 0; i < check_vector.nr; i++) {

556 + drop_attr_stack(&check_vector.checks[i]->stack);

557 + }

558 +

559 + vector_unlock();

560 +}

561 +

562 struct attr_check *attr_check_alloc(void)

563 {

483 - return xcalloc(1, sizeof(struct attr_check));

564 + struct attr_check *c = xcalloc(1, sizeof(struct attr_check));

565 +

566 + /* save pointer to the check struct */

567 + check_vector_add(c);

568 +

569 + return c;

570 }

571

572 struct attr_check *attr_check_initl(const char *one, ...)

@@ -543,12 +629,19 @@ void attr_check_clear(struct attr_check *check)

629 free(check->all_attrs);

630 check->all_attrs = NULL;

631 check->all_attrs_nr = 0;

632 +

633 + drop_attr_stack(&check->stack);

634 }

635

636 void attr_check_free(struct attr_check *check)

637 {

550 - attr_check_clear(check);

551 - free(check);

638 + if (check) {

639 + /* Remove check from the check vector */

640 + check_vector_remove(check);

641 +

642 + attr_check_clear(check);

643 + free(check);

644 + }

645 }

646

647 static const char *builtin_attr[] = {

@@ -705,15 +798,6 @@ static void debug_set(const char *what, const char *match, struct git_attr *attr

798 #define debug_set(a,b,c,d) do { ; } while (0)

799 #endif /* DEBUG_ATTR */

800

708 -static void drop_attr_stack(void)

709 -{

710 - while (attr_stack) {

711 - struct attr_stack *elem = attr_stack;

712 - attr_stack = elem->prev;

713 - free_attr_elem(elem);

714 - }

715 -}

716 -

801 static const char *git_etc_gitattributes(void)

802 {

803 static const char *system_wide;

@@ -722,6 +806,14 @@ static const char *git_etc_gitattributes(void)

806 return system_wide;

807 }

808

809 +static const char *get_home_gitattributes(void)

810 +{

811 + if (!git_attributes_file)

812 + git_attributes_file = xdg_config_home("attributes");

813 +

814 + return git_attributes_file;

815 +}

816 +

817 static int git_attr_system(void)

818 {

819 return !git_env_bool("GIT_ATTR_NOSYSTEM", 0);

@@ -741,47 +833,50 @@ static void push_stack(struct attr_stack **attr_stack_p,

833 }

834 }

835

744 -static void bootstrap_attr_stack(void)

836 +static void bootstrap_attr_stack(struct attr_stack **stack)

837 {

746 - struct attr_stack *elem;

838 + struct attr_stack *e;

839

748 - if (attr_stack)

840 + if (*stack)

841 return;

842

751 - push_stack(&attr_stack, read_attr_from_array(builtin_attr), NULL, 0);

752 -

753 - if (git_attr_system())

754 - push_stack(&attr_stack,

755 - read_attr_from_file(git_etc_gitattributes(), 1),

756 - NULL, 0);

843 + /* builtin frame */

844 + e = read_attr_from_array(builtin_attr);

845 + push_stack(stack, e, NULL, 0);

846

758 - if (!git_attributes_file)

759 - git_attributes_file = xdg_config_home("attributes");

760 - if (git_attributes_file)

761 - push_stack(&attr_stack,

762 - read_attr_from_file(git_attributes_file, 1),

763 - NULL, 0);

847 + /* system-wide frame */

848 + if (git_attr_system()) {

849 + e = read_attr_from_file(git_etc_gitattributes(), 1);

850 + push_stack(stack, e, NULL, 0);

851 + }

852

765 - if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {

766 - elem = read_attr(GITATTRIBUTES_FILE, 1);

767 - push_stack(&attr_stack, elem, xstrdup(""), 0);

768 - debug_push(elem);

853 + /* home directory */

854 + if (get_home_gitattributes()) {

855 + e = read_attr_from_file(get_home_gitattributes(), 1);

856 + push_stack(stack, e, NULL, 0);

857 }

858

771 - if (startup_info->have_repository)

772 - elem = read_attr_from_file(git_path_info_attributes(), 1);

859 + /* root directory */

860 + if (!is_bare_repository() || direction == GIT_ATTR_INDEX)

861 + e = read_attr(GITATTRIBUTES_FILE, 1);

862 else

774 - elem = NULL;

863 + e = xcalloc(1, sizeof(struct attr_stack));

864 + push_stack(stack, e, xstrdup(""), 0);

865

776 - if (!elem)

777 - elem = xcalloc(1, sizeof(*elem));

778 - push_stack(&attr_stack, elem, NULL, 0);

866 + /* info frame */

867 + if (startup_info->have_repository)

868 + e = read_attr_from_file(git_path_info_attributes(), 1);

869 + else

870 + e = NULL;

871 + if (!e)

872 + e = xcalloc(1, sizeof(struct attr_stack));

873 + push_stack(stack, e, NULL, 0);

874 }

875

781 -static void prepare_attr_stack(const char *path, int dirlen)

876 +static void prepare_attr_stack(const char *path, int dirlen,

877 + struct attr_stack **stack)

878 {

783 - struct attr_stack *elem, *info;

784 - const char *cp;

879 + struct attr_stack *info;

880

881 /*

882 * At the bottom of the attribute stack is the built-in

@@ -798,13 +893,13 @@ static void prepare_attr_stack(const char *path, int dirlen)

893 * .gitattributes in deeper directories to shallower ones,

894 * and finally use the built-in set as the default.

895 */

801 - bootstrap_attr_stack();

896 + bootstrap_attr_stack(stack);

897

898 /*

899 * Pop the "info" one that is always at the top of the stack.

900 */

806 - info = attr_stack;

807 - attr_stack = info->prev;

901 + info = *stack;

902 + *stack = info->prev;

903

904 /*

905 * Pop the ones from directories that are not the prefix of

@@ -812,18 +907,19 @@ static void prepare_attr_stack(const char *path, int dirlen)

907 * the root one (whose origin is an empty string "") or the builtin

908 * one (whose origin is NULL) without popping it.

909 */

815 - while (attr_stack->origin) {

816 - int namelen = strlen(attr_stack->origin);

910 + while ((*stack)->origin) {

911 + int namelen = (*stack)->originlen;

912 + struct attr_stack *elem;

913

818 - elem = attr_stack;

914 + elem = *stack;

915 if (namelen <= dirlen &&

916 !strncmp(elem->origin, path, namelen) &&

917 (!namelen || path[namelen] == '/'))

918 break;

919

920 debug_pop(elem);

825 - attr_stack = elem->prev;

826 - free_attr_elem(elem);

921 + *stack = elem->prev;

922 + attr_stack_free(elem);

923 }

924

925 /*

@@ -838,33 +934,43 @@ static void prepare_attr_stack(const char *path, int dirlen)

934 */

935 struct strbuf pathbuf = STRBUF_INIT;

936

841 - assert(attr_stack->origin);

842 - while (1) {

843 - size_t len = strlen(attr_stack->origin);

937 + assert((*stack)->origin);

938 + strbuf_addstr(&pathbuf, (*stack)->origin);

939 + /* Build up to the directory 'path' is in */

940 + while (pathbuf.len < dirlen) {

941 + size_t len = pathbuf.len;

942 + struct attr_stack *next;

943 char *origin;

944

846 - if (dirlen <= len)

847 - break;

848 - cp = memchr(path + len + 1, '/', dirlen - len - 1);

849 - if (!cp)

850 - cp = path + dirlen;

851 - strbuf_addf(&pathbuf,

852 - "%.*s/%s", (int)(cp - path), path,

853 - GITATTRIBUTES_FILE);

854 - elem = read_attr(pathbuf.buf, 0);

855 - strbuf_setlen(&pathbuf, cp - path);

856 - origin = strbuf_detach(&pathbuf, &len);

857 - push_stack(&attr_stack, elem, origin, len);

858 - debug_push(elem);

859 - }

945 + /* Skip path-separator */

946 + if (len < dirlen && is_dir_sep(path[len]))

947 + len++;

948 + /* Find the end of the next component */

949 + while (len < dirlen && !is_dir_sep(path[len]))

950 + len++;

951 +

952 + if (pathbuf.len > 0)

953 + strbuf_addch(&pathbuf, '/');

954 + strbuf_add(&pathbuf, path + pathbuf.len,

955 + (len - pathbuf.len));

956 + strbuf_addf(&pathbuf, "/%s", GITATTRIBUTES_FILE);

957 +

958 + next = read_attr(pathbuf.buf, 0);

959

960 + /* reset the pathbuf to not include "/.gitattributes" */

961 + strbuf_setlen(&pathbuf, len);

962 +

963 + origin = xstrdup(pathbuf.buf);

964 + push_stack(stack, next, origin, len);

965 +

966 + }

967 strbuf_release(&pathbuf);

968 }

969

970 /*

971 * Finally push the "info" one at the top of the stack.

972 */

867 - push_stack(&attr_stack, info, NULL, 0);

973 + push_stack(stack, info, NULL, 0);

974 }

975

976 static int path_matches(const char *pathname, int pathlen,

@@ -915,20 +1021,23 @@ static int fill_one(const char *what, struct all_attrs_item *all_attrs,

1021 }

1022

1023 static int fill(const char *path, int pathlen, int basename_offset,

918 - struct attr_stack *stk, struct all_attrs_item *all_attrs,

919 - int rem)

1024 + const struct attr_stack *stack,

1025 + struct all_attrs_item *all_attrs, int rem)

1026 {

921 - int i;

922 - const char *base = stk->origin ? stk->origin : "";

923 -

924 - for (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {

925 - const struct match_attr *a = stk->attrs[i];

926 - if (a->is_macro)

927 - continue;

928 - if (path_matches(path, pathlen, basename_offset,

929 - &a->u.pat, base, stk->originlen))

930 - rem = fill_one("fill", all_attrs, a, rem);

1027 + for (; rem > 0 && stack; stack = stack->prev) {

1028 + int i;

1029 + const char *base = stack->origin ? stack->origin : "";

1030 +

1031 + for (i = stack->num_matches - 1; 0 < rem && 0 <= i; i--) {

1032 + const struct match_attr *a = stack->attrs[i];

1033 + if (a->is_macro)

1034 + continue;

1035 + if (path_matches(path, pathlen, basename_offset,

1036 + &a->u.pat, base, stack->originlen))

1037 + rem = fill_one("fill", all_attrs, a, rem);

1038 + }

1039 }

1040 +

1041 return rem;

1042 }

1043

@@ -971,7 +1080,6 @@ static void determine_macros(struct all_attrs_item *all_attrs,

1080 */

1081 static void collect_some_attrs(const char *path, struct attr_check *check)

1082 {

974 - struct attr_stack *stk;

1083 int i, pathlen, rem, dirlen;

1084 const char *cp, *last_slash = NULL;

1085 int basename_offset;

@@ -989,9 +1097,9 @@ static void collect_some_attrs(const char *path, struct attr_check *check)

1097 dirlen = 0;

1098 }

1099

992 - prepare_attr_stack(path, dirlen);

1100 + prepare_attr_stack(path, dirlen, &check->stack);

1101 all_attrs_init(&g_attr_hashmap, check);

994 - determine_macros(check->all_attrs, attr_stack);

1102 + determine_macros(check->all_attrs, check->stack);

1103

1104 if (check->nr) {

1105 rem = 0;

@@ -1008,8 +1116,7 @@ static void collect_some_attrs(const char *path, struct attr_check *check)

1116 }

1117

1118 rem = check->all_attrs_nr;

1011 - for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)

1012 - rem = fill(path, pathlen, basename_offset, stk, check->all_attrs, rem);

1119 + fill(path, pathlen, basename_offset, check->stack, check->all_attrs, rem);

1120 }

1121

1122 int git_check_attr(const char *path, struct attr_check *check)

@@ -1056,7 +1163,7 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist

1163

1164 direction = new;

1165 if (new != old)

1059 - drop_attr_stack();

1166 + drop_all_attr_stacks();

1167 use_index = istate;

1168 }

1169

@@ -1064,5 +1171,6 @@ void attr_start(void)

1171 {

1172 #ifndef NO_PTHREADS

1173 pthread_mutex_init(&g_attr_hashmap.mutex, NULL);

1174 + pthread_mutex_init(&check_vector.mutex, NULL);

1175 #endif

1176 }

attr.h

+3 -1

@@ -4,8 +4,9 @@

4 /* An attribute is a pointer to this opaque structure */

5 struct git_attr;

7 -/* opaque structure used internally for attribute collection */

7 +/* opaque structures used internally for attribute collection */

8 struct all_attrs_item;

9 +struct attr_stack;

11 /*

12 * Given a string, return the gitattribute object that

@@ -38,6 +39,7 @@ struct attr_check {

39 struct attr_check_item *items;

40 int all_attrs_nr;

41 struct all_attrs_item *all_attrs;

42 + struct attr_stack *stack;

43 };

45 extern struct attr_check *attr_check_alloc(void);