Re: [PATCH] filter-branch: add passed/remaining seconds on progress

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gábor Bernát <bernat@xxxxxxxxxxxxxx> writes:

> +if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
> +then
> +	show_seconds=t
> +else
> +	show_seconds=
> +fi
> +
> +case "$show_seconds" in
> +t)
> +	start_timestamp=$(date +%s)
> +	next_sample_at=0
> +	;;
> +'')
> +	progress=""
> +	;;
> +esac

Why the case statement here?

For that matter, you probably do not need $show_seconds and use the
fact that start_timestamp and progress are only set to a non-empty
string when we measure progress, i.e. making all of the above
something more like this:

        progress= start_timestamp=
        if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
        then
                next_sample_at=0
                progress="dummy to ensure this is not empty"
                start_timestamp=$(date '+%s')
        fi

If you are more efficiency-minded, I think you could even do with a
single call to /bin/date, perhaps like so:

        next_sample_at= progress=
        start_timestamp=$(date '+%s' 2>/dev/null) 
        case "$start_timestamp" in
        *[!0-9]* | "")
                # not a "digit only" output
                ;;
        ?*)
                progress="dummy to ensure this is not empty"
                next_sample_at=0
                ;;
        esac

but I suspect that may be going a bit too far ;-).

>  while read commit parents; do
>  	git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
> -	printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)"
> +
> +	case "$show_seconds" in
> +	t)
> +		if test $git_filter_branch__commit_count -gt $next_sample_at
> +		then
> +			now_timestamp=$(date +%s)
> +			elapsed_seconds=$(($now_timestamp - $start_timestamp))
> +			remaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))
> +			if test $elapsed_seconds -gt 0
> +			then
> +				next_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))
> +			else
> +				next_sample_at=$(($next_sample_at + 1))
> +			fi
> +			progress=" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)"
> +		fi
> +		;;
> +	'')
> +		progress=""
> +		;;
> +	esac
> +
> +	printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress"
>  
>  	case "$filter_subdir" in
>  	"")

It would be easier to follow the logic of this loop whose _primary_
point is to rewrite one commit if you moved this part into a helper
function.  As written above, the deeply nested case statement whose
purpose is only to show the progress is very distracting.

Then the loop would read like:

        while read commit parents
        do
                : $(( $git_filter_branch__commit_count++ ))
                report_progress

                case "$filter_subdir" in
                ...

                # all the work that is about rewriting this commit
                # comes here.

        done

which would be much less noisy and helpful to people who are
interested in learning what it is doing.

As I said earlier in this message, the report_progress helper
function can use the fact that $progress or $start_timestamp are
non-empty if and only if you need to compute and tack $progress at
the end of the output, i.e.

        report_progress ()
        {
                if test -n "$progress"
                then
                        ... do rate computation here ...
                        progress=" ($elapsed_seconds seconds passed,..."
                fi
                printf "\rRewrite $commit (...)$progress"
        }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]