1. 22 4月, 2014 6 次提交
  2. 18 4月, 2014 1 次提交
  3. 08 4月, 2014 2 次提交
    • K
      combine-diff: speed it up, by using multiparent diff tree-walker directly · 7195fbfa
      Kirill Smelkov 提交于
      As was recently shown in "combine-diff: optimize
      combine_diff_path sets intersection", combine-diff runs very slowly. In
      that commit we optimized paths sets intersection, but that accounted
      only for ~ 25% of the slowness, and as my tracing showed, for linux.git
      v3.10..v3.11, for merges a lot of time is spent computing
      diff(commit,commit^2) just to only then intersect that huge diff to
      almost small set of files from diff(commit,commit^1).
      
      In previous commit, we described the problem in more details, and
      reworked the diff tree-walker to be general one - i.e. to work in
      multiple parent case too. Now is the time to take advantage of it for
      finding paths for combine diff.
      
      The implementation is straightforward - if we know, we can get generated
      diff paths directly, and at present that means no diff filtering or
      rename/copy detection was requested(*), we can call multiparent tree-walker
      directly and get ready paths.
      
      (*) because e.g. at present, all diffcore transformations work on
          diff_filepair queues, but in the future, that limitation can be
          lifted, if filters would operate directly on combine_diff_paths.
      
      Timings for `git log --raw --no-abbrev --no-renames` without `-c` ("git log")
      and with `-c` ("git log -c") and with `-c --merges` ("git log -c --merges")
      before and after the patch are as follows:
      
                      linux.git v3.10..v3.11
      
                  log     log -c     log -c --merges
      
          before  1.9s    16.4s      15.2s
          after   1.9s     2.4s       1.1s
      
      The result stayed the same.
      Signed-off-by: NKirill Smelkov <kirr@mns.spb.ru>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      7195fbfa
    • K
      tree-diff: rework diff_tree() to generate diffs for multiparent cases as well · 72441af7
      Kirill Smelkov 提交于
      Previously diff_tree(), which is now named ll_diff_tree_sha1(), was
      generating diff_filepair(s) for two trees t1 and t2, and that was
      usually used for a commit as t1=HEAD~, and t2=HEAD - i.e. to see changes
      a commit introduces.
      
      In Git, however, we have fundamentally built flexibility in that a
      commit can have many parents - 1 for a plain commit, 2 for a simple merge,
      but also more than 2 for merging several heads at once.
      
      For merges there is a so called combine-diff, which shows diff, a merge
      introduces by itself, omitting changes done by any parent. That works
      through first finding paths, that are different to all parents, and then
      showing generalized diff, with separate columns for +/- for each parent.
      The code lives in combine-diff.c .
      
      There is an impedance mismatch, however, in that a commit could
      generally have any number of parents, and that while diffing trees, we
      divide cases for 2-tree diffs and more-than-2-tree diffs. I mean there
      is no special casing for multiple parents commits in e.g.
      revision-walker .
      
      That impedance mismatch *hurts* *performance* *badly* for generating
      combined diffs - in "combine-diff: optimize combine_diff_path
      sets intersection" I've already removed some slowness from it, but from
      the timings provided there, it could be seen, that combined diffs still
      cost more than an order of magnitude more cpu time, compared to diff for
      usual commits, and that would only be an optimistic estimate, if we take
      into account that for e.g. linux.git there is only one merge for several
      dozens of plain commits.
      
      That slowness comes from the fact that currently, while generating
      combined diff, a lot of time is spent computing diff(commit,commit^2)
      just to only then intersect that huge diff to almost small set of files
      from diff(commit,commit^1).
      
      That's because at present, to compute combine-diff, for first finding
      paths, that "every parent touches", we use the following combine-diff
      property/definition:
      
      D(A,P1...Pn) = D(A,P1) ^ ... ^ D(A,Pn)      (w.r.t. paths)
      
      where
      
      D(A,P1...Pn) is combined diff between commit A, and parents Pi
      
      and
      
      D(A,Pi) is usual two-tree diff Pi..A
      
      So if any of that D(A,Pi) is huge, tracting 1 n-parent combine-diff as n
      1-parent diffs and intersecting results will be slow.
      
      And usually, for linux.git and other topic-based workflows, that
      D(A,P2) is huge, because, if merge-base of A and P2, is several dozens
      of merges (from A, via first parent) below, that D(A,P2) will be diffing
      sum of merges from several subsystems to 1 subsystem.
      
      The solution is to avoid computing n 1-parent diffs, and to find
      changed-to-all-parents paths via scanning A's and all Pi's trees
      simultaneously, at each step comparing their entries, and based on that
      comparison, populate paths result, and deduce we could *skip*
      *recursing* into subdirectories, if at least for 1 parent, sha1 of that
      dir tree is the same as in A. That would save us from doing significant
      amount of needless work.
      
      Such approach is very similar to what diff_tree() does, only there we
      deal with scanning only 2 trees simultaneously, and for n+1 tree, the
      logic is a bit more complex:
      
      D(T,P1...Pn) calculation scheme
      -------------------------------
      
      D(T,P1...Pn) = D(T,P1) ^ ... ^ D(T,Pn)	(regarding resulting paths set)
      
          D(T,Pj)		- diff between T..Pj
          D(T,P1...Pn)	- combined diff from T to parents P1,...,Pn
      
      We start from all trees, which are sorted, and compare their entries in
      lock-step:
      
           T     P1       Pn
           -     -        -
          |t|   |p1|     |pn|
          |-|   |--| ... |--|      imin = argmin(p1...pn)
          | |   |  |     |  |
          |-|   |--|     |--|
          |.|   |. |     |. |
           .     .        .
           .     .        .
      
      at any time there could be 3 cases:
      
          1)  t < p[imin];
          2)  t > p[imin];
          3)  t = p[imin].
      
      Schematic deduction of what every case means, and what to do, follows:
      
      1)  t < p[imin]  ->  ∀j t ∉ Pj  ->  "+t" ∈ D(T,Pj)  ->  D += "+t";  t↓
      
      2)  t > p[imin]
      
          2.1) ∃j: pj > p[imin]  ->  "-p[imin]" ∉ D(T,Pj)  ->  D += ø;  ∀ pi=p[imin]  pi↓
          2.2) ∀i  pi = p[imin]  ->  pi ∉ T  ->  "-pi" ∈ D(T,Pi)  ->  D += "-p[imin]";  ∀i pi↓
      
      3)  t = p[imin]
      
          3.1) ∃j: pj > p[imin]  ->  "+t" ∈ D(T,Pj)  ->  only pi=p[imin] remains to investigate
          3.2) pi = p[imin]  ->  investigate δ(t,pi)
           |
           |
           v
      
          3.1+3.2) looking at δ(t,pi) ∀i: pi=p[imin] - if all != ø  ->
      
                            ⎧δ(t,pi)  - if pi=p[imin]
                   ->  D += ⎨
                            ⎩"+t"     - if pi>p[imin]
      
          in any case t↓  ∀ pi=p[imin]  pi↓
      
      ~
      
      For comparison, here is how diff_tree() works:
      
      D(A,B) calculation scheme
      -------------------------
      
          A     B
          -     -
         |a|   |b|    a < b   ->  a ∉ B   ->   D(A,B) +=  +a    a↓
         |-|   |-|    a > b   ->  b ∉ A   ->   D(A,B) +=  -b    b↓
         | |   | |    a = b   ->  investigate δ(a,b)            a↓ b↓
         |-|   |-|
         |.|   |.|
          .     .
          .     .
      
      ~~~~~~~~
      
      This patch generalizes diff tree-walker to work with arbitrary number of
      parents as described above - i.e. now there is a resulting tree t, and
      some parents trees tp[i] i=[0..nparent). The generalization builds on
      the fact that usual diff
      
      D(A,B)
      
      is by definition the same as combined diff
      
      D(A,[B]),
      
      so if we could rework the code for common case and make it be not slower
      for nparent=1 case, usual diff(t1,t2) generation will not be slower, and
      multiparent diff tree-walker would greatly benefit generating
      combine-diff.
      
      What we do is as follows:
      
      1) diff tree-walker ll_diff_tree_sha1() is internally reworked to be
         a paths generator (new name diff_tree_paths()), with each generated path
         being `struct combine_diff_path` with info for path, new sha1,mode and for
         every parent which sha1,mode it was in it.
      
      2) From that info, we can still generate usual diff queue with
         struct diff_filepairs, via "exporting" generated
         combine_diff_path, if we know we run for nparent=1 case.
         (see emit_diff() which is now named emit_diff_first_parent_only())
      
      3) In order for diff_can_quit_early(), which checks
      
             DIFF_OPT_TST(opt, HAS_CHANGES))
      
         to work, that exporting have to be happening not in bulk, but
         incrementally, one diff path at a time.
      
         For such consumers, there is a new callback in diff_options
         introduced:
      
             ->pathchange(opt, struct combine_diff_path *)
      
         which, if set to !NULL, is called for every generated path.
      
         (see new compat ll_diff_tree_sha1() wrapper around new paths
          generator for setup)
      
      4) The paths generation itself, is reworked from previous
         ll_diff_tree_sha1() code according to "D(A,P1...Pn) calculation
         scheme" provided above:
      
         On the start we allocate [nparent] arrays in place what was
         earlier just for one parent tree.
      
         then we just generalize loops, and comparison according to the
         algorithm.
      
      Some notes(*):
      
      1) alloca(), for small arrays, is used for "runs not slower for
         nparent=1 case than before" goal - if we change it to xmalloc()/free()
         the timings get ~1% worse. For alloca() we use just-introduced
         xalloca/xalloca_free compatibility wrappers, so it should not be a
         portability problem.
      
      2) For every parent tree, we need to keep a tag, whether entry from that
         parent equals to entry from minimal parent. For performance reasons I'm
         keeping that tag in entry's mode field in unused bit - see S_IFXMIN_NEQ.
         Not doing so, we'd need to alloca another [nparent] array, which hurts
         performance.
      
      3) For emitted paths, memory could be reused, if we know the path was
         processed via callback and will not be needed later. We use efficient
         hand-made realloc-style path_appendnew(), that saves us from ~1-1.5%
         of potential additional slowdown.
      
      4) goto(s) are used in several places, as the code executes a little bit
         faster with lowered register pressure.
      
      Also
      
      - we should now check for FIND_COPIES_HARDER not only when two entries
        names are the same, and their hashes are equal, but also for a case,
        when a path was removed from some of all parents having it.
      
        The reason is, if we don't, that path won't be emitted at all (see
        "a > xi" case), and we'll just skip it, and FIND_COPIES_HARDER wants
        all paths - with diff or without - to be emitted, to be later analyzed
        for being copies sources.
      
        The new check is only necessary for nparent >1, as for nparent=1 case
        xmin_eqtotal always =1 =nparent, and a path is always added to diff as
        removal.
      
      ~~~~~~~~
      
      Timings for
      
          # without -c, i.e. testing only nparent=1 case
          `git log --raw --no-abbrev --no-renames`
      
      before and after the patch are as follows:
      
                      navy.git        linux.git v3.10..v3.11
      
          before      0.611s          1.889s
          after       0.619s          1.907s
          slowdown    1.3%            0.9%
      
      This timings show we did no harm to usual diff(tree1,tree2) generation.
      From the table we can see that we actually did ~1% slowdown, but I think
      I've "earned" that 1% in the previous patch ("tree-diff: reuse base
      str(buf) memory on sub-tree recursion", HEAD~~) so for nparent=1 case,
      net timings stays approximately the same.
      
      The output also stayed the same.
      
      (*) If we revert 1)-4) to more usual techniques, for nparent=1 case,
          we'll get ~2-2.5% of additional slowdown, which I've tried to avoid, as
         "do no harm for nparent=1 case" rule.
      
      For linux.git, combined diff will run an order of magnitude faster and
      appropriate timings will be provided in the next commit, as we'll be
      taking advantage of the new diff tree-walker for combined-diff
      generation there.
      
      P.S. and combined diff is not some exotic/for-play-only stuff - for
      example for a program I write to represent Git archives as readonly
      filesystem, there is initial scan with
      
          `git log --reverse --raw --no-abbrev --no-renames -c`
      
      to extract log of what was created/changed when, as a result building a
      map
      
          {}  sha1    ->  in which commit (and date) a content was added
      
      that `-c` means also show combined diff for merges, and without them, if
      a merge is non-trivial (merges changes from two parents with both having
      separate changes to a file), or an evil one, the map will not be full,
      i.e. some valid sha1 would be absent from it.
      
      That case was my initial motivation for combined diffs speedup.
      Signed-off-by: NKirill Smelkov <kirr@mns.spb.ru>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      72441af7
  4. 01 4月, 2014 1 次提交
  5. 10 3月, 2014 1 次提交
  6. 04 3月, 2014 1 次提交
  7. 25 2月, 2014 2 次提交
  8. 19 2月, 2014 1 次提交
  9. 18 1月, 2014 1 次提交
  10. 19 12月, 2013 1 次提交
  11. 17 12月, 2013 1 次提交
  12. 07 12月, 2013 1 次提交
    • Z
      difftool: display the number of files in the diff queue in the prompt · ee7fb0b1
      Zoltan Klinger 提交于
      When --prompt option is set, git-difftool displays a prompt for each
      modified file to be viewed in an external diff program.  At that
      point, it could be useful to display a counter and the total number
      of files in the diff queue.
      
      Below is the current difftool prompt for the first of 5 modified files:
      
          Viewing: 'diff.c'
          Launch 'vimdiff' [Y/n]:
      
      Consider the modified prompt:
      
          Viewing (1/5): 'diff.c'
          Launch 'vimdiff' [Y/n]:
      
      The current GIT_EXTERNAL_DIFF mechanism does not tell the number of
      paths in the diff queue nor the current counter.  To make this
      "counter/total" info available for GIT_EXTERNAL_DIFF programs
      without breaking existing ones by doing the following:
      
       - Keep track of the number of paths shown so far in diff_options;
      
       - Export two new environment variables from run_external_diff() to
         show the total number of paths (from diff_queue_struct) and the
         current value of the counter (from diff_options); and
      
       - Update git-difftool--helper to use these two environment variables.
      Signed-off-by: NZoltan Klinger <zoltan.klinger@gmail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      ee7fb0b1
  13. 06 12月, 2013 1 次提交
    • C
      replace {pre,suf}fixcmp() with {starts,ends}_with() · 59556548
      Christian Couder 提交于
      Leaving only the function definitions and declarations so that any
      new topic in flight can still make use of the old functions, replace
      existing uses of the prefixcmp() and suffixcmp() with new API
      functions.
      
      The change can be recreated by mechanically applying this:
      
          $ git grep -l -e prefixcmp -e suffixcmp -- \*.c |
            grep -v strbuf\\.c |
            xargs perl -pi -e '
              s|!prefixcmp\(|starts_with\(|g;
              s|prefixcmp\(|!starts_with\(|g;
              s|!suffixcmp\(|ends_with\(|g;
              s|suffixcmp\(|!ends_with\(|g;
            '
      
      on the result of preparatory changes in this series.
      Signed-off-by: NChristian Couder <chriscool@tuxfamily.org>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      59556548
  14. 01 11月, 2013 1 次提交
  15. 10 8月, 2013 2 次提交
    • S
      diff: fix a possible null pointer dereference · 3b0c18af
      Stefan Beller 提交于
      The condition in the ternary operator was wrong, hence the wrong char
      pointer could be used as the parameter for show_submodule_summary.
      one->path may be null, but we definitely need a non null path given
      to the function.
      Signed-off-by: NStefan Beller <stefanbeller@googlemail.com>
      Acked-By: NJohannes Schindelin <Johannes.Schindelin@gmx.de>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      3b0c18af
    • S
      diff: remove ternary operator evaluating always to true · c189c4f2
      Stefan Beller 提交于
      The line being changed is deep inside the function builtin_diff.
      The variable name_b, which is used to evaluate the ternary expression
      must evaluate to true at that position, hence the replacement with
      just name_b.
      
      The name_b variable only occurs a few times in that lengthy function:
      As a parameter to the function itself:
      	static void builtin_diff(const char *name_a,
      				 const char *name_b,
      				...
      The next occurrences are at:
      	/* Never use a non-valid filename anywhere if at all possible */
      	name_a = DIFF_FILE_VALID(one) ? name_a : name_b;
      	name_b = DIFF_FILE_VALID(two) ? name_b : name_a;
      
      	a_one = quote_two(a_prefix, name_a + (*name_a == '/'));
      	b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
      
      In the last line of this block 'name_b' is dereferenced and compared
      to '/'. This would crash if name_b was NULL. Hence in the following code
      we can assume name_b being non-null.
      
      The next occurrence is just as a function argument, which doesn't change
      the memory, which name_b points to, so the assumption name_b being not
      null still holds:
      	emit_rewrite_diff(name_a, name_b, one, two,
      				textconv_one, textconv_two, o);
      
      The next occurrence would be the line of this patch. As name_b still must
      be not null, we can remove the ternary operator.
      
      Inside the emit_rewrite_diff function there is a also a line
      	ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
      which was also simplified as there is also a dereference before the
      ternary operator.
      Signed-off-by: NStefan Beller <stefanbeller@googlemail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      c189c4f2
  16. 23 7月, 2013 1 次提交
  17. 20 7月, 2013 2 次提交
  18. 18 7月, 2013 7 次提交
  19. 16 7月, 2013 1 次提交
  20. 10 7月, 2013 1 次提交
    • N
      Convert "struct cache_entry *" to "const ..." wherever possible · 9c5e6c80
      Nguyễn Thái Ngọc Duy 提交于
      I attempted to make index_state->cache[] a "const struct cache_entry **"
      to find out how existing entries in index are modified and where. The
      question I have is what do we do if we really need to keep track of on-disk
      changes in the index. The result is
      
       - diff-lib.c: setting CE_UPTODATE
      
       - name-hash.c: setting CE_HASHED
      
       - preload-index.c, read-cache.c, unpack-trees.c and
         builtin/update-index: obvious
      
       - entry.c: write_entry() may refresh the checked out entry via
         fill_stat_cache_info(). This causes "non-const struct cache_entry
         *" in builtin/apply.c, builtin/checkout-index.c and
         builtin/checkout.c
      
       - builtin/ls-files.c: --with-tree changes stagemask and may set
         CE_UPDATE
      
      Of these, write_entry() and its call sites are probably most
      interesting because it modifies on-disk info. But this is stat info
      and can be retrieved via refresh, at least for porcelain
      commands. Other just uses ce_flags for local purposes.
      
      So, keeping track of "dirty" entries is just a matter of setting a
      flag in index modification functions exposed by read-cache.c. Except
      unpack-trees, the rest of the code base does not do anything funny
      behind read-cache's back.
      
      The actual patch is less valueable than the summary above. But if
      anyone wants to re-identify the above sites. Applying this patch, then
      this:
      
          diff --git a/cache.h b/cache.h
          index 430d021..1692891 100644
          --- a/cache.h
          +++ b/cache.h
          @@ -267,7 +267,7 @@ static inline unsigned int canon_mode(unsigned int mode)
           #define cache_entry_size(len) (offsetof(struct cache_entry,name) + (len) + 1)
      
           struct index_state {
          -	struct cache_entry **cache;
          +	const struct cache_entry **cache;
           	unsigned int version;
           	unsigned int cache_nr, cache_alloc, cache_changed;
           	struct string_list *resolve_undo;
      
      will help quickly identify them without bogus warnings.
      Signed-off-by: NNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      9c5e6c80
  21. 26 6月, 2013 1 次提交
  22. 20 6月, 2013 1 次提交
    • A
      diff: add --ignore-blank-lines option · 36617af7
      Antoine Pelisse 提交于
      The goal of the patch is to introduce the GNU diff
      -B/--ignore-blank-lines as closely as possible. The short option is not
      available because it's already used for "break-rewrites".
      
      When this option is used, git-diff will not create hunks that simply
      add or remove empty lines, but will still show empty lines
      addition/suppression if they are close enough to "valuable" changes.
      
      There are two differences between this option and GNU diff -B option:
      - GNU diff doesn't have "--inter-hunk-context", so this must be handled
      - The following sequence looks like a bug (context is displayed twice):
      
          $ seq 5 >file1
          $ cat <<EOF >file2
          change
          1
          2
      
          3
          4
          5
          change
          EOF
          $ diff -u -B file1 file2
          --- file1	2013-06-08 22:13:04.471517834 +0200
          +++ file2	2013-06-08 22:13:23.275517855 +0200
          @@ -1,5 +1,7 @@
          +change
           1
           2
          +
           3
           4
           5
          @@ -3,3 +5,4 @@
           3
           4
           5
          +change
      
      So here is a more thorough description of the option:
      - real changes are interesting
      - blank lines that are close enough (less than context size) to
      interesting changes are considered interesting (recursive definition)
      - "context" lines are used around each hunk of interesting changes
      - If two hunks are separated by less than "inter-hunk-context", they
      will be merged into one.
      
      The implementation does the "interesting changes selection" in a single
      pass.
      Signed-off-by: NAntoine Pelisse <apelisse@gmail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      36617af7
  23. 11 5月, 2013 1 次提交
    • J
      diff_opt: track whether flags have been set explicitly · 6c374008
      Junio C Hamano 提交于
      The diff_opt infrastructure sets flags based on defaults and command
      line options.  It is impossible to tell whether a flag has been set
      as a default or on explicit request.  Update the structure so that
      this detection is possible:
      
       * Add an extra "opt->touched_flags" that keeps track of all the
         fields that have been touched by DIFF_OPT_SET and DIFF_OPT_CLR.
      
       * You may continue setting the default values to the flags, like
         commands in the "log" family do in cmd_log_init_defaults(), but
         after you finished setting the defaults, you clear the
         touched_flags field;
      
       * And then you let the usual callchain call diff_opt_parse(),
         allowing the opt->flags be set or unset, while keeping track of
         which bits the user touched;
      
       * There is an optional callback "opt->set_default" that is called
         at the very beginning to let you inspect touched_flags and update
         opt->flags appropriately, before the remainder of the diffcore
         machinery is set up, taking the opt->flags value into account.
      Signed-off-by: NMichael J Gruber <git@drmicha.warpmail.net>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      6c374008
  24. 13 4月, 2013 1 次提交
  25. 06 4月, 2013 1 次提交