Per some of the discussion online and off I locally broke up up the question
and answer and I wasn't very thrilled with the outcome for a number of reasons.
1. The API is more complex. Callers needs to have two structures allocated
instead of one, one can be shared read-only while the other can't. While this
many not be that big of a deal, it was more confusing to me.
2. Performance hit. The allocation churn with creating/freeing a
scoreboard and the results struct adds up. It even looks like the
cost of looking up a stack frame in a hashmap isn't very cheap.
Here are some very rough performance measurements I made on my machine
on linux.git by: `perf stat -r 50 git grep "asdfghjkl"`
master: 0.302176063 seconds
v1: 0.324243806 seconds
v2: 0.304339636 seconds
split: 0.349892023 seconds (hashtable of stacks, all_attr scoreboard
allocated per git_attr_check() call, split
question/answer)
After looking at this, I'm of the opinion that the API in v2 is the best route
to take. Its a step-up from what it is currently (at master) and there isn't a
performance degradation (ok there's a small bit but it seems within the margin
of error). It also allows for easier adaptation of the API if we wanted to do
a change in the future since the primary functionality remains intact, or to do
optimizations like stack pruning (if we decided to go down that route).
Given the above, v3 is a reroll of the same design as in v2. This is a good
milestone in improving the attribute system as it achieves the goal of making
the attribute subsystem thread-safe (ie multiple callers can be executing
inside the attribute system at the same time) and will enable a future series
to allow pathspec code to call into the attribute system.
Most of the changes in this revision are cosmetic (variable renames, code
movement, etc) but there was a memory leak that was also fixed.
Brandon Williams (8):
attr: pass struct attr_check to collect_some_attrs
attr: use hashmap for attribute dictionary
attr: eliminate global check_all_attr array
attr: remove maybe-real, maybe-macro from git_attr
attr: tighten const correctness with git_attr and match_attr
attr: store attribute stack in attr_check structure
attr: push the bare repo check into read_attr()
attr: reformat git_attr_set_direction() function
Junio C Hamano (17):
commit.c: use strchrnul() to scan for one line
attr.c: use strchrnul() to scan for one line
attr.c: update a stale comment on "struct match_attr"
attr.c: explain the lack of attr-name syntax check in parse_attr()
attr.c: complete a sentence in a comment
attr.c: mark where #if DEBUG ends more clearly
attr.c: simplify macroexpand_one()
attr.c: tighten constness around "git_attr" structure
attr.c: plug small leak in parse_attr_line()
attr.c: add push_stack() helper
attr.c: outline the future plans by heavily commenting
attr: rename function and struct related to checking attributes
attr: (re)introduce git_check_attr() and struct attr_check
attr: convert git_all_attrs() to use "struct attr_check"
attr: convert git_check_attrs() callers to use the new API
attr: retire git_check_attrs() API
attr: change validity check for attribute names to use positive logic
Nguyễn Thái Ngọc Duy (1):
attr: support quoting pathname patterns in C style
Stefan Beller (1):
Documentation: fix a typo
Documentation/gitattributes.txt | 10 +-
Documentation/technical/api-gitattributes.txt | 86 ++-
archive.c | 24 +-
attr.c | 878 ++++++++++++++++++--------
attr.h | 49 +-
builtin/check-attr.c | 66 +-
builtin/pack-objects.c | 19 +-
commit.c | 3 +-
common-main.c | 3 +
convert.c | 25 +-
ll-merge.c | 33 +-
t/t0003-attributes.sh | 26 +
userdiff.c | 19 +-
ws.c | 19 +-
14 files changed, 816 insertions(+), 444 deletions(-)
--
2.11.0.483.g087da7b7c-goog
