| Message ID | 20190319190317.6632-1-phillip.wood123@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | rebase -i run without forking rebase--interactive | expand |
On 2019.03.19 19:03, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the builtin rebase starts an interactive rebase it parses the > options and then repackages them and forks `rebase--interactive`. This > series refactors rebase--interactive so that interactive rebases can > be started by the builtin rebase without forking. My motivation was to > make it easier to debug the sequencer but this should help future > maintainability. > > This series involves some code movement so viewing the diffs with > --color-moved is recommended. > > These patches are based on a merge of master [e902e9bcae ("The second > batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 > ("rebase--interactive: move transform_todo_file()", 2019-03-05). They > can be fetched from the tag rebase-i-no-fork/rfc at > https://github.com/phillipwood/git.git > > Phillip Wood (11): > sequencer: always discard index after checkout > rebase: rename write_basic_state() > rebase: use OPT_RERERE_AUTOUPDATE() > rebase -i: combine rebase--interactive.c with rebase.c > rebase -i: remove duplication > rebase -i: use struct commit when parsing options > rebase -i: use struct object_id for squash_onto > rebase -i: use struct rebase_options to parse args > rebase -i: use struct rebase_options in do_interactive_rebase() > rebase: use a common action enum > rebase -i: run without forking rebase--interactive Although I'm not very familiar with the rebase code, this series looks good to me. Reviewed-by: Josh Steadmon <steadmon@google.com>
On Tue, Mar 19 2019, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the builtin rebase starts an interactive rebase it parses the > options and then repackages them and forks `rebase--interactive`. This > series refactors rebase--interactive so that interactive rebases can > be started by the builtin rebase without forking. My motivation was to > make it easier to debug the sequencer but this should help future > maintainability. > > This series involves some code movement so viewing the diffs with > --color-moved is recommended. > > These patches are based on a merge of master [e902e9bcae ("The second > batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 > ("rebase--interactive: move transform_todo_file()", 2019-03-05). They > can be fetched from the tag rebase-i-no-fork/rfc at > https://github.com/phillipwood/git.git Just a that the t/perf/*rebase* numbers look much better with this. I don't have these in front of me anymore, but over 10 runs with -O3 one of those long-runnings test was 30% faster. Another one (rebase -i) went from 0.02 to 0.01 sec, with that short amount of time I wonder (but didn't dig) if the test itself is broken...
Phillip Wood <phillip.wood123@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the builtin rebase starts an interactive rebase it parses the > options and then repackages them and forks `rebase--interactive`. This > series refactors rebase--interactive so that interactive rebases can > be started by the builtin rebase without forking. My motivation was to > make it easier to debug the sequencer but this should help future > maintainability. > > This series involves some code movement so viewing the diffs with > --color-moved is recommended. > > These patches are based on a merge of master [e902e9bcae ("The second > batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 > ("rebase--interactive: move transform_todo_file()", 2019-03-05). They > can be fetched from the tag rebase-i-no-fork/rfc at > https://github.com/phillipwood/git.git Exciting. > > Phillip Wood (11): > sequencer: always discard index after checkout > rebase: rename write_basic_state() > rebase: use OPT_RERERE_AUTOUPDATE() > rebase -i: combine rebase--interactive.c with rebase.c > rebase -i: remove duplication > rebase -i: use struct commit when parsing options > rebase -i: use struct object_id for squash_onto > rebase -i: use struct rebase_options to parse args > rebase -i: use struct rebase_options in do_interactive_rebase() > rebase: use a common action enum > rebase -i: run without forking rebase--interactive > > Makefile | 1 - > builtin/rebase--interactive.c | 377 -------------------- > builtin/rebase.c | 625 ++++++++++++++++++++++++++-------- > parse-options-cb.c | 34 ++ > parse-options.h | 4 + > sequencer.c | 42 ++- > sequencer.h | 7 +- > 7 files changed, 556 insertions(+), 534 deletions(-) > delete mode 100644 builtin/rebase--interactive.c
On 20/03/2019 23:05, Ævar Arnfjörð Bjarmason wrote: > > On Tue, Mar 19 2019, Phillip Wood wrote: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> When the builtin rebase starts an interactive rebase it parses the >> options and then repackages them and forks `rebase--interactive`. This >> series refactors rebase--interactive so that interactive rebases can >> be started by the builtin rebase without forking. My motivation was to >> make it easier to debug the sequencer but this should help future >> maintainability. >> >> This series involves some code movement so viewing the diffs with >> --color-moved is recommended. >> >> These patches are based on a merge of master [e902e9bcae ("The second >> batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 >> ("rebase--interactive: move transform_todo_file()", 2019-03-05). They >> can be fetched from the tag rebase-i-no-fork/rfc at >> https://github.com/phillipwood/git.git > > Just a that the t/perf/*rebase* numbers look much better with this. I > don't have these in front of me anymore, but over 10 runs with -O3 one > of those long-runnings test was 30% faster. That's surprising I wouldn't expect that much change as the time taken to fork shouldn't be that significant compared to the overall running time. I'll make a note to have a look at those tests. > Another one (rebase -i) went from 0.02 to 0.01 sec, with that short > amount of time I wonder (but didn't dig) if the test itself is broken... Yeah, that sounds suspicious Thanks for sharing those results Phillip
From: Phillip Wood <phillip.wood@dunelm.org.uk> When the builtin rebase starts an interactive rebase it parses the options and then repackages them and forks `rebase--interactive`. This series refactors rebase--interactive so that interactive rebases can be started by the builtin rebase without forking. My motivation was to make it easier to debug the sequencer but this should help future maintainability. This series involves some code movement so viewing the diffs with --color-moved is recommended. These patches are based on a merge of master [e902e9bcae ("The second batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 ("rebase--interactive: move transform_todo_file()", 2019-03-05). They can be fetched from the tag rebase-i-no-fork/rfc at https://github.com/phillipwood/git.git Phillip Wood (11): sequencer: always discard index after checkout rebase: rename write_basic_state() rebase: use OPT_RERERE_AUTOUPDATE() rebase -i: combine rebase--interactive.c with rebase.c rebase -i: remove duplication rebase -i: use struct commit when parsing options rebase -i: use struct object_id for squash_onto rebase -i: use struct rebase_options to parse args rebase -i: use struct rebase_options in do_interactive_rebase() rebase: use a common action enum rebase -i: run without forking rebase--interactive Makefile | 1 - builtin/rebase--interactive.c | 377 -------------------- builtin/rebase.c | 625 ++++++++++++++++++++++++++-------- parse-options-cb.c | 34 ++ parse-options.h | 4 + sequencer.c | 42 ++- sequencer.h | 7 +- 7 files changed, 556 insertions(+), 534 deletions(-) delete mode 100644 builtin/rebase--interactive.c