Re: [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase
Hi Thomas, On Sun, 20 Mar 2016, Thomas Gummerer wrote: > On 03/18, Johannes Schindelin wrote: > > > > On Fri, 18 Mar 2016, Thomas Gummerer wrote: > > > > > [1] > > > http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gumme...@gmail.com > > > > Yes, I agree that something like that is needed. The proposed commit > > message suggests that things get simpler, though, while I would > > contend that timings get more accurate. > > > > And I think you could simply move the test_start command, but that's > > just from a *very* cursory reading of the patch. > > Is it possible that you might have missed patch 2/2 [1]? Yes, I did not read that patch. Sorry for the noise. Ciao, Dscho -- 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/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase
On 03/18, Johannes Schindelin wrote: > Hi Thomas, > > On Fri, 18 Mar 2016, Thomas Gummerer wrote: > > > On 03/16, Johannes Schindelin wrote: > > > Hrm. rebase -f just makes the reset an implicit part of the rebase, so it > > > seems we cannot perf *just* the rebase. We are stuck with perf'ing also > > > the reset. Sad. > > > > I had the same problem back when I was working on index-v5 and posted > > a patch series. The discussion about it is at [1]. Maybe it could be > > worth resurrecting? > > > > [1] > > http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gumme...@gmail.com > > Yes, I agree that something like that is needed. The proposed commit > message suggests that things get simpler, though, while I would contend > that timings get more accurate. > > And I think you could simply move the test_start command, but that's just > from a *very* cursory reading of the patch. Is it possible that you might have missed patch 2/2 [1]? The first patch there is only the preparation for making the timings more acurate when some cleanup is involved. Just moving the test_start command wouldn't do anything for the timings to get more acurate, as the timings are measured in the test_run_perf_ function (it's outside of the diff in the patch series). I'm also not sure whether the test_perf_cleanup or if the other series in the thread that came out of a suggestion by Junio makes more sense [2]. (That is minus patch 3/3 in that series, which was added so we could have a user of the new series, but if it's going to be used here that's unnecessary). [1] http://thread.gmane.org/gmane.comp.version-control.git/234874/focus=234875 [2] http://thread.gmane.org/gmane.comp.version-control.git/234874/focus=235241 > Ciao, > Dscho -- Thomas -- 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/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase
On 03/16, Johannes Schindelin wrote: > Hi Paul, > > On Wed, 16 Mar 2016, Paul Tan wrote: > > > On Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin > > wrote: > > > > > > On Sat, 12 Mar 2016, Paul Tan wrote: > > > > > >> diff --git a/t/perf/p3404-rebase-interactive.sh > > >> b/t/perf/p3404-rebase-interactive.sh > > >> new file mode 100755 > > >> index 000..aaca105 > > >> --- /dev/null > > >> +++ b/t/perf/p3404-rebase-interactive.sh > > >> @@ -0,0 +1,26 @@ > > >> > > >> [...] > > >> > > >> +test_perf 'rebase -i --onto master^' ' > > >> + git checkout perf-topic-branch && > > >> + git reset --hard perf-topic-branch-initial && > > >> + GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master > > >> +' > > > > > > This measures the performance of checkout && reset && rebase -i. Maybe we > > > should only test rebase -i? > > > > test_perf runs the same script multiple times, so we need to reset > > --hard at least to undo the changes of the rebase. > > > > I think we can remove the reset if we use rebase -f and rebase onto > > the same base, but -f was not implemented in this patch series. > > Hrm. rebase -f just makes the reset an implicit part of the rebase, so it > seems we cannot perf *just* the rebase. We are stuck with perf'ing also > the reset. Sad. I had the same problem back when I was working on index-v5 and posted a patch series. The discussion about it is at [1]. Maybe it could be worth resurrecting? [1] http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gumme...@gmail.com -- Thomas -- 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/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase
Hi Thomas, On Fri, 18 Mar 2016, Thomas Gummerer wrote: > On 03/16, Johannes Schindelin wrote: > > > > On Wed, 16 Mar 2016, Paul Tan wrote: > > > > > On Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin > > > wrote: > > > > > > > > On Sat, 12 Mar 2016, Paul Tan wrote: > > > > > > > >> diff --git a/t/perf/p3404-rebase-interactive.sh > > > >> b/t/perf/p3404-rebase-interactive.sh > > > >> new file mode 100755 > > > >> index 000..aaca105 > > > >> --- /dev/null > > > >> +++ b/t/perf/p3404-rebase-interactive.sh > > > >> @@ -0,0 +1,26 @@ > > > >> > > > >> [...] > > > >> > > > >> +test_perf 'rebase -i --onto master^' ' > > > >> + git checkout perf-topic-branch && > > > >> + git reset --hard perf-topic-branch-initial && > > > >> + GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master > > > >> +' > > > > > > > > This measures the performance of checkout && reset && rebase -i. Maybe > > > > we > > > > should only test rebase -i? > > > > > > test_perf runs the same script multiple times, so we need to reset > > > --hard at least to undo the changes of the rebase. > > > > > > I think we can remove the reset if we use rebase -f and rebase onto > > > the same base, but -f was not implemented in this patch series. > > > > Hrm. rebase -f just makes the reset an implicit part of the rebase, so it > > seems we cannot perf *just* the rebase. We are stuck with perf'ing also > > the reset. Sad. > > I had the same problem back when I was working on index-v5 and posted > a patch series. The discussion about it is at [1]. Maybe it could be > worth resurrecting? > > [1] > http://thread.gmane.org/gmane.comp.version-control.git/1379419842-32627-1-git-send-email-t.gumme...@gmail.com Yes, I agree that something like that is needed. The proposed commit message suggests that things get simpler, though, while I would contend that timings get more accurate. And I think you could simply move the test_start command, but that's just from a *very* cursory reading of the patch. Ciao, Dscho -- 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/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase
Hi Paul, On Wed, 16 Mar 2016, Paul Tan wrote: > On Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin > wrote: > > > > On Sat, 12 Mar 2016, Paul Tan wrote: > > > >> diff --git a/t/perf/p3404-rebase-interactive.sh > >> b/t/perf/p3404-rebase-interactive.sh > >> new file mode 100755 > >> index 000..aaca105 > >> --- /dev/null > >> +++ b/t/perf/p3404-rebase-interactive.sh > >> @@ -0,0 +1,26 @@ > >> > >> [...] > >> > >> +test_perf 'rebase -i --onto master^' ' > >> + git checkout perf-topic-branch && > >> + git reset --hard perf-topic-branch-initial && > >> + GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master > >> +' > > > > This measures the performance of checkout && reset && rebase -i. Maybe we > > should only test rebase -i? > > test_perf runs the same script multiple times, so we need to reset > --hard at least to undo the changes of the rebase. > > I think we can remove the reset if we use rebase -f and rebase onto > the same base, but -f was not implemented in this patch series. Hrm. rebase -f just makes the reset an implicit part of the rebase, so it seems we cannot perf *just* the rebase. We are stuck with perf'ing also the reset. Sad. > > Also, I would strongly recommend an extra test_commit after reset; > > Otherwise you would only test the logic that verifies that it can simply > > fast-forward instead of cherry-picking. > > Or, we could use the -f flag, I think. Yeah, we can do that, too. Ciao, Dscho -- 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/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase
Hi Dscho, On Wed, Mar 16, 2016 at 3:58 PM, Johannes Schindelin wrote: > Hi Paul, > > On Sat, 12 Mar 2016, Paul Tan wrote: > >> diff --git a/t/perf/p3404-rebase-interactive.sh >> b/t/perf/p3404-rebase-interactive.sh >> new file mode 100755 >> index 000..aaca105 >> --- /dev/null >> +++ b/t/perf/p3404-rebase-interactive.sh >> @@ -0,0 +1,26 @@ >> >> [...] >> >> +test_perf 'rebase -i --onto master^' ' >> + git checkout perf-topic-branch && >> + git reset --hard perf-topic-branch-initial && >> + GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master >> +' > > This measures the performance of checkout && reset && rebase -i. Maybe we > should only test rebase -i? test_perf runs the same script multiple times, so we need to reset --hard at least to undo the changes of the rebase. I think we can remove the reset if we use rebase -f and rebase onto the same base, but -f was not implemented in this patch series. > Also, I would strongly recommend an extra test_commit after reset; > Otherwise you would only test the logic that verifies that it can simply > fast-forward instead of cherry-picking. Or, we could use the -f flag, I think. Thanks, Paul -- 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/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase
Hi Paul, On Sat, 12 Mar 2016, Paul Tan wrote: > diff --git a/t/perf/p3404-rebase-interactive.sh > b/t/perf/p3404-rebase-interactive.sh > new file mode 100755 > index 000..aaca105 > --- /dev/null > +++ b/t/perf/p3404-rebase-interactive.sh > @@ -0,0 +1,26 @@ > > [...] > > +test_perf 'rebase -i --onto master^' ' > + git checkout perf-topic-branch && > + git reset --hard perf-topic-branch-initial && > + GIT_SEQUENCE_EDITOR=: git rebase -i --onto master^ master > +' This measures the performance of checkout && reset && rebase -i. Maybe we should only test rebase -i? Also, I would strongly recommend an extra test_commit after reset; Otherwise you would only test the logic that verifies that it can simply fast-forward instead of cherry-picking. Ciao, Dscho -- 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
