Thomas Rast <tr...@student.ethz.ch> writes:

> Test                      with patch        before
> --------------------------------------------------------------------
> 4000.2: log --raw -3000   0.50(0.43+0.06)   0.54(0.46+0.06) +7.0%***
> 4000.3: log -p -3000      2.34(2.20+0.13)   2.37(2.22+0.13) +1.2%
> --------------------------------------------------------------------
> Significance hints:  '.' 0.1  '*' 0.05  '**' 0.01  '***' 0.001

It may be a silly question but what is a significance hint?

> Signed-off-by: Thomas Rast <tr...@student.ethz.ch>
> ---
>  log-tree.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index eb1a1b4..277a38f 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -715,7 +715,7 @@ static int log_tree_diff(struct rev_info *opt, struct 
> commit *commit, struct log
>  {
>       int showed_log;
>       struct commit_list *parents;
> -     unsigned const char *sha1 = commit->object.sha1;
> +     unsigned const char *sha1 = commit->tree->object.sha1;

Overall I think this goes in the right direction and I can see why
the changes in later two hunks are correct, but I am not sure if we
can safely assume that the caller has parsed the incoming commit and
we have a good commit->tree here already.

Right now, this static function has a single call-site in a public
function log_tree_commit(), whose existing callers may all pass an
already parsed commit, but I feel somewhat uneasy to do the above
without some mechanism in place (either parse it here or in the
caller if unparsed, or document that log_tree_commit() must be
called with a parsed commit and perhaps add an assert there) to
ensure that the invariant is not broken in the future.

Thanks.

>       if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
>               return 0;
> @@ -742,7 +742,8 @@ static int log_tree_diff(struct rev_info *opt, struct 
> commit *commit, struct log
>                        * parent, showing summary diff of the others
>                        * we merged _in_.
>                        */
> -                     diff_tree_sha1(parents->item->object.sha1, sha1, "", 
> &opt->diffopt);
> +                     parse_commit(parents->item);
> +                     diff_tree_sha1(parents->item->tree->object.sha1, sha1, 
> "", &opt->diffopt);
>                       log_tree_diff_flush(opt);
>                       return !opt->loginfo;
>               }
> @@ -755,7 +756,8 @@ static int log_tree_diff(struct rev_info *opt, struct 
> commit *commit, struct log
>       for (;;) {
>               struct commit *parent = parents->item;
>  
> -             diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt);
> +             parse_commit(parent);
> +             diff_tree_sha1(parent->tree->object.sha1, sha1, "", 
> &opt->diffopt);
>               log_tree_diff_flush(opt);
>  
>               showed_log |= !opt->loginfo;
--
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

Reply via email to