Re: [PATCH v3 5/9] revision.c: Make --full-history consider more merges

2013-05-07 Thread Junio C Hamano
Kevin Bracey  writes:

> On 06/05/2013 23:45, Junio C Hamano wrote:
>> Kevin Bracey  writes:
>>
>>> +struct treesame_state {
>>> +   unsigned int nparents;
>>> +   unsigned char treesame[FLEX_ARRAY];
>>> +};
>> I have been wondering if we want to do one-bit (not one-byte) per
>> parent but no biggie ;-)
>
> I did start down that path, because I felt bad about bloat.
>
> But then I realised how much I would be complicating and slowing the
> code down to only save a few bytes each time we walk a merge with at
> least 5 parents, and I came to my senses. :)

;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/9] revision.c: Make --full-history consider more merges

2013-05-07 Thread Kevin Bracey

On 06/05/2013 23:45, Junio C Hamano wrote:

Kevin Bracey  writes:


+struct treesame_state {
+   unsigned int nparents;
+   unsigned char treesame[FLEX_ARRAY];
+};

I have been wondering if we want to do one-bit (not one-byte) per
parent but no biggie ;-)


I did start down that path, because I felt bad about bloat.

But then I realised how much I would be complicating and slowing the 
code down to only save a few bytes each time we walk a merge with at 
least 5 parents, and I came to my senses. :)


Kevin


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/9] revision.c: Make --full-history consider more merges

2013-05-06 Thread Junio C Hamano
Kevin Bracey  writes:

> diff --git a/revision.c b/revision.c
> index a67b615..c88ded8 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -429,10 +429,100 @@ static int rev_same_tree_as_empty(struct rev_info 
> *revs, struct commit *commit)
>   return retval >= 0 && (tree_difference == REV_TREE_SAME);
>  }
>  
> +struct treesame_state {
> + unsigned int nparents;
> + unsigned char treesame[FLEX_ARRAY];
> +};

I have been wondering if we want to do one-bit (not one-byte) per
parent but no biggie ;-)

> @@ -1971,6 +2083,70 @@ static struct merge_simplify_state 
> *locate_simplify_state(struct rev_info *revs,
>   return st;
>  }
>  
> +static int mark_redundant_parents(struct rev_info *revs, struct commit 
> *commit)
> +{
> +...
> + po=po->next;

po = po->next;

> + }

This seems to be identical (modulo tests) from the previous round,
which I found a reasonable thing to do.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 5/9] revision.c: Make --full-history consider more merges

2013-05-05 Thread Kevin Bracey
History simplification previously always treated merges as TREESAME
if they were TREESAME to any parent.

While this was consistent with the default behaviour, this could be
extremely unhelpful when searching detailed history, and could not be
overridden. For example, if a merge had ignored a change, as if by "-s
ours", then:

  git log -m -p --full-history -Schange file

would successfully locate "change"'s addition but would not locate the
merge that resolved against it.

Futher, simplify_merges could drop the actual parent that a commit
was TREESAME to, leaving it as a normal commit marked TREESAME that
isn't actually TREESAME to its remaining parent.

Now redefine a commit's TREESAME flag to be true only if a commit is
TREESAME to _all_ of its parents. This doesn't affect either the default
simplify_history behaviour (because partially TREESAME merges are turned
into normal commits), or full-history with parent rewriting (because all
merges are output). But it does affect other modes. The clearest
difference is that --full-history will show more merges - sufficient to
ensure that -m -p --full-history log searches can really explain every
change to the file, including those changes' ultimate fate in merges.

Also modify simplify_merges to recalculate TREESAME after removing
a parent. This is achieved by storing per-parent TREESAME flags on the
initial scan, so the combined flag can be easily recomputed.

This fixes some t6111 failures, but creates a couple of new ones -
we are now showing some merges that don't need to be shown.

Signed-off-by: Kevin Bracey 
---
 Documentation/rev-list-options.txt |   6 +-
 revision.c | 241 -
 revision.h |   1 +
 t/t6019-rev-list-ancestry-path.sh  |   2 +-
 t/t6111-rev-list-treesame.sh   |  20 +--
 5 files changed, 226 insertions(+), 44 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 50bbff7..1dad341 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -409,10 +409,10 @@ parent lines.
the example, we get
 +
 ---
-   I  A  B  N  D  O
+   I  A  B  N  D  O  P
 ---
 +
-`P` and `M` were excluded because they are TREESAME to a parent.  `E`,
+`M` was excluded because it is TREESAME to both parents.  `E`,
 `C` and `B` were all walked, but only `B` was !TREESAME, so the others
 do not appear.
 +
@@ -440,7 +440,7 @@ themselves.  This results in
 Compare to '\--full-history' without rewriting above.  Note that `E`
 was pruned away because it is TREESAME, but the parent list of P was
 rewritten to contain `E`'s parent `I`.  The same happened for `C` and
-`N`.  Note also that `P` was included despite being TREESAME.
+`N`.
 
 In addition to the above settings, you can change whether TREESAME
 affects inclusion:
diff --git a/revision.c b/revision.c
index a67b615..c88ded8 100644
--- a/revision.c
+++ b/revision.c
@@ -429,10 +429,100 @@ static int rev_same_tree_as_empty(struct rev_info *revs, 
struct commit *commit)
return retval >= 0 && (tree_difference == REV_TREE_SAME);
 }
 
+struct treesame_state {
+   unsigned int nparents;
+   unsigned char treesame[FLEX_ARRAY];
+};
+
+static struct treesame_state *initialise_treesame(struct rev_info *revs, 
struct commit *commit)
+{
+   unsigned n = commit_list_count(commit->parents);
+   struct treesame_state *st = xcalloc(1, sizeof(*st) + n);
+   st->nparents = n;
+   add_decoration(&revs->treesame, &commit->object, st);
+   return st;
+}
+
+/*
+ * Must be called immediately after removing the nth_parent from a commit's
+ * parent list, if we are maintaining the per-parent treesame[] decoration.
+ * This does not recalculate the master TREESAME flag - update_treesame()
+ * should be called to update it after a sequence of treesame[] modifications
+ * that may have affected it.
+ */
+static int compact_treesame(struct rev_info *revs, struct commit *commit, 
unsigned nth_parent)
+{
+   struct treesame_state *st;
+   int old_same;
+
+   if (!commit->parents) {
+   /*
+* Have just removed the only parent from a non-merge.
+* Different handling, as we lack decoration.
+*/
+   if (nth_parent != 0)
+   die("compact_treesame %u", nth_parent);
+   old_same = !!(commit->object.flags & TREESAME);
+   if (rev_same_tree_as_empty(revs, commit))
+   commit->object.flags |= TREESAME;
+   else
+   commit->object.flags &= ~TREESAME;
+   return old_same;
+   }
+
+   st = lookup_decoration(&revs->treesame, &commit->object);
+   if (!st || nth_parent >= st->nparents)
+   die("compact_tree