{"thread":{"id":"41502","subject":"Rebase performance","startedAt":"2016-02-24T22:09:12Z","lastAt":"2016-03-02T10:13:44Z","messageCount":13,"participants":["Christian Couder","Jacob Keller","Stefan Beller","Duy Nguyen","Junio C Hamano","Ævar Arnfjörð Bjarmason","Matthieu Moy","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"279184","messageId":"CAP8UFD0p1kvk2B0kkc-M9dm+H-Bmam=OrE99VwQx=KCETFEjcw@mail.gmail.com","threadId":"41502","inReplyTo":null,"subject":"Rebase performance","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-02-24T22:09:12Z","receivedAt":"2016-02-24T22:09:12Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nUsing GIT_TRACE_PERFORMANCE it looks like a lot of time in a regular\nrebase is spent in run_apply() in builtin/am.c. This function first\nsets up a 'struct child_process cp' to launch \"git apply\" on a patch\nand then uses run_command(&cp) to actually launch the \"git apply\".\nThen this function calls discard_cache() and read_cache_from() to get\nthe index created by the \"git apply\".\n\nOn a Linux server with many CPUs and many cores on each CPU, it is\nstrange because the same rebase of 13 commits on a big repo is\nsignificantly slower than on a laptop (typically around 9 seconds\nversus 6 seconds). Both the server and the laptop have that has SSD\nstorage.\n\nIt appears that the server is trying to run the \"git apply\" processes\non different cores or cpus perhaps to try to spread the load on many\nof its cores. Anyway adding something like \"taskset -c 7\" in front of\nthe \"git rebase...\" command, when launching it on the server, speeds\nit up, so that it takes around the same amount of time as it does on\nthe laptop (6 seconds). \"taskset -c 7\" is just asking Linux to run a\nprocess and its children on core number 7, and it appears that doing\nthat results in a much better cpu (or core) cache usage which explains\nthe speed up.\n\nIf there was a config option called maybe \"rebase.taskset\" or\n\"rebase.setcpuaffinity\" that could be set to ask the OS for all the\nrebase child processes to be run on the same core, people who run many\nrebases on big repos on big servers as we do at Booking.com could\neasily benefit from a nice speed up.\n\nTechnically the option may make git-rebase--am.sh call \"git am\" using\n\"taskset\" (if taskset is available on the current OS).\n\nAnother possibility would be to libify the \"git apply\" functionality\nand then to use the libified \"git apply\" in run_apply() instead of\nlaunching a separate \"git apply\" process. One benefit from this is\nthat we could probably get rid of the read_cache_from() call at the\nend of run_apply() and this would likely further speed up things. Also\navoiding to launch separate processes might be a win especially on\nWindows.\n\nSuggestions?\n\nThanks,\nChristian.\n"},{"id":"279238","messageId":"CA+P7+xpC+gZijkcxcp8uFCmYjQf7-0xbBf7FWSsFnMXB1b7G+Q@mail.gmail.com","threadId":"41502","inReplyTo":"CAP8UFD0p1kvk2B0kkc-M9dm+H-Bmam=OrE99VwQx=KCETFEjcw@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-02-25T00:15:53Z","receivedAt":"2016-02-25T00:15:53Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Feb 24, 2016 at 2:09 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> Hi,\n> Another possibility would be to libify the \"git apply\" functionality\n> and then to use the libified \"git apply\" in run_apply() instead of\n> launching a separate \"git apply\" process. One benefit from this is\n> that we could probably get rid of the read_cache_from() call at the\n> end of run_apply() and this would likely further speed up things. Also\n> avoiding to launch separate processes might be a win especially on\n> Windows.\n>\n\nThis is the route I would go, since the addition of a taskset option\nseems pretty difficult to get correct, and may not be portable. This\nabove solution likely improves more in general, and is more portable.\nNot exactly sure how easy it would be to \"libify\" the required bits of\napply, however.. it may be that this is already available as well but\nwe just didn't go that route during the port of git-am into C code.\n\nRegards,\nJake\n"},{"id":"279241","messageId":"CAGZ79kauMxxXcRu97CZ=uyau-fpNpZ-8q8vM5fDRmUmuLaMLPw@mail.gmail.com","threadId":"41502","inReplyTo":"CA+P7+xpC+gZijkcxcp8uFCmYjQf7-0xbBf7FWSsFnMXB1b7G+Q@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T00:22:47Z","receivedAt":"2016-02-25T00:22:47Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Feb 24, 2016 at 4:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:\n> On Wed, Feb 24, 2016 at 2:09 PM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>> Hi,\n>> Another possibility would be to libify the \"git apply\" functionality\n>> and then to use the libified \"git apply\" in run_apply() instead of\n>> launching a separate \"git apply\" process. One benefit from this is\n>> that we could probably get rid of the read_cache_from() call at the\n>> end of run_apply() and this would likely further speed up things. Also\n>> avoiding to launch separate processes might be a win especially on\n>> Windows.\n>>\n>\n> This is the route I would go, since the addition of a taskset option\n> seems pretty difficult to get correct, and may not be portable. This\n> above solution likely improves more in general, and is more portable.\n> Not exactly sure how easy it would be to \"libify\" the required bits of\n> apply, however.. it may be that this is already available as well but\n> we just didn't go that route during the port of git-am into C code.\n>\n> Regards,\n> Jake\n\nIIRC Junio started libifying am after Paul Tan rewrote it in C,\nSee origin/jc/am-mailinfo-direct (tip at 4b98bae2cbc6b).\n\nThat part however only libifyies the mailinfo which is used\nby apply (one layer below), so I am not sure if the run_apply\nhas been attempted to libify.\n\nI would also encourage to rather try to not call out to a child\n(libifying) instead of adding the taskset hack for servers.\n\nThanks,\nStefan\n\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"279245","messageId":"CACsJy8DZOv2Z2hiUmRKHr_GCjsyVz9kQEE8a1F=h6Ku0Dr9g_w@mail.gmail.com","threadId":"41502","inReplyTo":"CAP8UFD0p1kvk2B0kkc-M9dm+H-Bmam=OrE99VwQx=KCETFEjcw@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-25T00:50:03Z","receivedAt":"2016-02-25T00:50:03Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 25, 2016 at 5:09 AM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> Another possibility would be to libify the \"git apply\" functionality\n> and then to use the libified \"git apply\" in run_apply() instead of\n> launching a separate \"git apply\" process. One benefit from this is\n> that we could probably get rid of the read_cache_from() call at the\n> end of run_apply() and this would likely further speed up things. Also\n> avoiding to launch separate processes might be a win especially on\n> Windows.\n\nThe amount of global variables in apply.c is just scary. Libification\nwill need some cleanup first (i'm not against it though). Out of\ncuriosity, how long does it take to do \"git update-index <one modified\npath>\" on this repo? That would cover read_cache_from() and\nwrite_cache(). While you're measuring, maybe sprinkle some\ntrace_performance() in apply.c:apply_patch() to get an idea where time\nis most spent in? This is a big guess, but if we spend more time\nparsing/validating the patch than applying, then that part could be\nparallelized, we only need to apply patches in sequence.\n-- \nDuy\n"},{"id":"279277","messageId":"xmqqvb5dpk5z.fsf@gitster.mtv.corp.google.com","threadId":"41502","inReplyTo":"CACsJy8DZOv2Z2hiUmRKHr_GCjsyVz9kQEE8a1F=h6Ku0Dr9g_w@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T03:02:00Z","receivedAt":"2016-02-25T03:02:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Thu, Feb 25, 2016 at 5:09 AM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>> Another possibility would be to libify the \"git apply\" functionality\n>> and then to use the libified \"git apply\" in run_apply() instead of\n>> launching a separate \"git apply\" process. One benefit from this is\n>> that we could probably get rid of the read_cache_from() call at the\n>> end of run_apply() and this would likely further speed up things. Also\n>> avoiding to launch separate processes might be a win especially on\n>> Windows.\n>\n> The amount of global variables in apply.c is just scary. Libification\n> will need some cleanup first (i'm not against it though).\n\nThe global variables do not scare me.  You can just throw them into\na single \"apply_state\" structure and pass it around the callchain\njust fine--the helper functions in the file all take only a small\nnumber of parameters, and a new parameter that is a pointer to the\nstate structure that consistently comes at the beginning would not\nmake things harder to read or maintain.\n\nWhat does scare me (and what you should be scared) more is the way\nthe current code handles erroneous input.  It was one of the\nprograms written in early days with \"just do one thing well and\nexit, the larger program structure will be scripted around us\"\nmentality and liberally die() without cleaning things up.  I do\nagree with others that libification of it is a very good thing, but\nthe second step (the first step is the easier \"global to state\nstructure\" one, which should be more or less mechanical) towards it\nis to design how the errors are handled and resources are released.\n"},{"id":"279276","messageId":"CACsJy8DMyD9oo2_3+BdM-M=0eYAi1ip77WTuYt42tBn9DuD+kw@mail.gmail.com","threadId":"41502","inReplyTo":"xmqqvb5dpk5z.fsf@gitster.mtv.corp.google.com","subject":"Re: Rebase performance","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-25T03:14:02Z","receivedAt":"2016-02-25T03:14:02Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 25, 2016 at 10:02 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Duy Nguyen <pclouds@gmail.com> writes:\n>\n>> On Thu, Feb 25, 2016 at 5:09 AM, Christian Couder\n>> <christian.couder@gmail.com> wrote:\n>>> Another possibility would be to libify the \"git apply\" functionality\n>>> and then to use the libified \"git apply\" in run_apply() instead of\n>>> launching a separate \"git apply\" process. One benefit from this is\n>>> that we could probably get rid of the read_cache_from() call at the\n>>> end of run_apply() and this would likely further speed up things. Also\n>>> avoiding to launch separate processes might be a win especially on\n>>> Windows.\n>>\n>> The amount of global variables in apply.c is just scary. Libification\n>> will need some cleanup first (i'm not against it though).\n>\n> The global variables do not scare me.  You can just throw them into\n> a single \"apply_state\" structure and pass it around the callchain\n> just fine--the helper functions in the file all take only a small\n> number of parameters, and a new parameter that is a pointer to the\n> state structure that consistently comes at the beginning would not\n> make things harder to read or maintain.\n\nThere are a bunch of shadow variables in this file. If you are not\ncareful, it's easy to mistake local var \"x\" with global \"x\" and\nreplace it with global_state->x.\n\n> What does scare me (and what you should be scared) more is the way\n> the current code handles erroneous input.  It was one of the\n> programs written in early days with \"just do one thing well and\n> exit, the larger program structure will be scripted around us\"\n> mentality and liberally die() without cleaning things up.  I do\n> agree with others that libification of it is a very good thing, but\n> the second step (the first step is the easier \"global to state\n> structure\" one, which should be more or less mechanical) towards it\n> is to design how the errors are handled and resources are released.\n\nYeah.. thorough study of apply code may be needed before anybody does\nany surgery in this code\n-- \nDuy\n"},{"id":"279305","messageId":"CACsJy8AUbLhfkeanQa101mygt1u_SxgbVAGc=USC79n9iR4R3w@mail.gmail.com","threadId":"41502","inReplyTo":"CACsJy8DZOv2Z2hiUmRKHr_GCjsyVz9kQEE8a1F=h6Ku0Dr9g_w@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-25T09:42:28Z","receivedAt":"2016-02-25T09:42:28Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 25, 2016 at 7:50 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Thu, Feb 25, 2016 at 5:09 AM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>> Another possibility would be to libify the \"git apply\" functionality\n>> and then to use the libified \"git apply\" in run_apply() instead of\n>> launching a separate \"git apply\" process. One benefit from this is\n>> that we could probably get rid of the read_cache_from() call at the\n>> end of run_apply() and this would likely further speed up things. Also\n>> avoiding to launch separate processes might be a win especially on\n>> Windows.\n>\n> The amount of global variables in apply.c is just scary. Libification\n> will need some cleanup first (i'm not against it though). Out of\n> curiosity, how long does it take to do \"git update-index <one modified\n> path>\" on this repo? That would cover read_cache_from() and\n> write_cache(). While you're measuring, maybe sprinkle some\n> trace_performance() in apply.c:apply_patch() to get an idea where time\n> is most spent in?\n\nI did some experiment. The calls from am are basically\n\nfor i in $PATCHES; do git apply --cached $i; git commit; done\n\nand we can approximate the libification version of that with\n\ngit apply --cached $PATCHES\n\n(I hacked git-apply to do write-tree after each patch, close enough to\ngit-commit).\n\nI tried it on my shallow-deepen series, 26 patches. The \"for; do\ngit-apply;done\" command took 0m0.482s (real's time), taskset does not\naffect much for me, while the \"libification\" took just  0m0.105s.\nThat's a very impressive number to aim for, and git.git is a small\nrepo.\n-- \nDuy\n"},{"id":"279345","messageId":"CACBZZX7rVAdzfCm=0FdrCXSx8a2=a8n7pjq1ZSW-V3fzmaSGWw@mail.gmail.com","threadId":"41502","inReplyTo":"CAP8UFD0p1kvk2B0kkc-M9dm+H-Bmam=OrE99VwQx=KCETFEjcw@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-02-25T16:31:04Z","receivedAt":"2016-02-25T16:31:04Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Feb 24, 2016 at 11:09 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n\n[Resent because I was accidentally in GMail's HTML mode and the ML rejected it]\n\n> If there was a config option called maybe \"rebase.taskset\" or\n> \"rebase.setcpuaffinity\" that could be set to ask the OS for all the\n> rebase child processes to be run on the same core, people who run many\n> rebases on big repos on big servers as we do at Booking.com could\n> easily benefit from a nice speed up.\n>\n> Technically the option may make git-rebase--am.sh call \"git am\" using\n> \"taskset\" (if taskset is available on the current OS).\n\nI think aside from issues with git-apply this would be an interesting\nfeature to have in git. I.e. some general facility to intercept\ncommands and inject a prefix command in front of them, whether that's\ntaskset, nice/ionice, strace etc.\n\n> Another possibility would be to libify the \"git apply\" functionality\n> and then to use the libified \"git apply\" in run_apply() instead of\n> launching a separate \"git apply\" process. One benefit from this is\n> that we could probably get rid of the read_cache_from() call at the\n> end of run_apply() and this would likely further speed up things. Also\n> avoiding to launch separate processes might be a win especially on\n> Windows.\n\nYeah that should help in this particular case and make the taskset\nredundant since the whole sequence of operations would all be on one\ncore, right?\n\nAt the risk of derailing this thread, a thing that would make rebase\neven faster I think would be to change it so that instead of applying\na patch at a time to the working tree the whole operation takes place\non temporary trees & commits and then we'll eventually move the branch\npointer to that once it's finished.\n\nI.e. there's no reason for why a sequence of 1000 patches where a\nFOO.txt is changed from \"hi1\", \"hi2\", \"hi3\", ... would be noticeably\nslower than applying the same changes with git-fast-import.\n\nOf course this would require a lot of nuances, e.g. if there's a\nconflict we'd need to change the working tree & index as we do now\nbefore continuing.\n\nHas anyone looked into some advanced refactoring of the rebase process\nthat would work like this, or has some feedback on why this would be\ndumb or that there's a better way to do it?\n"},{"id":"279349","messageId":"vpq37sg4s0l.fsf@anie.imag.fr","threadId":"41502","inReplyTo":"CACBZZX7rVAdzfCm=0FdrCXSx8a2=a8n7pjq1ZSW-V3fzmaSGWw@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-02-25T17:30:18Z","receivedAt":"2016-02-25T17:30:18Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> At the risk of derailing this thread, a thing that would make rebase\n> even faster I think would be to change it so that instead of applying\n> a patch at a time to the working tree the whole operation takes place\n> on temporary trees & commits and then we'll eventually move the branch\n> pointer to that once it's finished.\n>\n> I.e. there's no reason for why a sequence of 1000 patches where a\n> FOO.txt is changed from \"hi1\", \"hi2\", \"hi3\", ... would be noticeably\n> slower than applying the same changes with git-fast-import.\n\nAlso, not touching the worktree during rebase would have the advantage\nthat if the final result doesn't change a file, we wouldn't need to\ntouch this file at all, hence the next \"make\" (or whatever\ntimestamp-using build system the user runs) would consider this file\nunchanged.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"279524","messageId":"alpine.DEB.2.20.1602261644570.3152@virtualbox","threadId":"41502","inReplyTo":"vpq37sg4s0l.fsf@anie.imag.fr","subject":"Re: Rebase performance","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-02-26T15:45:31Z","receivedAt":"2016-02-26T15:45:31Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Matthieu,\n\nOn Thu, 25 Feb 2016, Matthieu Moy wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n> \n> > At the risk of derailing this thread, a thing that would make rebase\n> > even faster I think would be to change it so that instead of applying\n> > a patch at a time to the working tree the whole operation takes place\n> > on temporary trees & commits and then we'll eventually move the branch\n> > pointer to that once it's finished.\n> >\n> > I.e. there's no reason for why a sequence of 1000 patches where a\n> > FOO.txt is changed from \"hi1\", \"hi2\", \"hi3\", ... would be noticeably\n> > slower than applying the same changes with git-fast-import.\n> \n> Also, not touching the worktree during rebase would have the advantage\n> that if the final result doesn't change a file, we wouldn't need to\n> touch this file at all, hence the next \"make\" (or whatever\n> timestamp-using build system the user runs) would consider this file\n> unchanged.\n\nWe still have to write all blobs. So I would still expect this to be I/O\nbound.\n\nCiao,\nDscho"},{"id":"279540","messageId":"CAGZ79ka5qTGuLOz3mMr0s=8yOF7LR0iRDhf9jy38GBppOhzD6Q@mail.gmail.com","threadId":"41502","inReplyTo":"alpine.DEB.2.20.1602261644570.3152@virtualbox","subject":"Re: Rebase performance","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-26T17:15:05Z","receivedAt":"2016-02-26T17:15:05Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Feb 26, 2016 at 7:45 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Matthieu,\n>\n> On Thu, 25 Feb 2016, Matthieu Moy wrote:\n>\n>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>>\n>> > At the risk of derailing this thread, a thing that would make rebase\n>> > even faster I think would be to change it so that instead of applying\n>> > a patch at a time to the working tree the whole operation takes place\n>> > on temporary trees & commits and then we'll eventually move the branch\n>> > pointer to that once it's finished.\n>> >\n>> > I.e. there's no reason for why a sequence of 1000 patches where a\n>> > FOO.txt is changed from \"hi1\", \"hi2\", \"hi3\", ... would be noticeably\n>> > slower than applying the same changes with git-fast-import.\n>>\n>> Also, not touching the worktree during rebase would have the advantage\n>> that if the final result doesn't change a file, we wouldn't need to\n>> touch this file at all, hence the next \"make\" (or whatever\n>> timestamp-using build system the user runs) would consider this file\n>> unchanged.\n>\n> We still have to write all blobs. So I would still expect this to be I/O\n> bound.\n\nBut if there is an IO bound process, the only way to make it faster\nis to reduce its IO, which was the proposal here? I agree that it probably\nis not enough to shift it from being IO bound to say CPU bounded.\n\nThanks,\nStefan\n\n>\n> Ciao,\n> Dscho\n"},{"id":"279546","messageId":"CAP8UFD0_+8cQvBHvrEe16DEa6F-4KZtJGKNqiRn_fMgfQqX_cg@mail.gmail.com","threadId":"41502","inReplyTo":"CACsJy8AUbLhfkeanQa101mygt1u_SxgbVAGc=USC79n9iR4R3w@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-02-26T18:15:12Z","receivedAt":"2016-02-26T18:15:12Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Feb 25, 2016 at 10:42 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Thu, Feb 25, 2016 at 7:50 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n>> On Thu, Feb 25, 2016 at 5:09 AM, Christian Couder\n>> <christian.couder@gmail.com> wrote:\n>>> Another possibility would be to libify the \"git apply\" functionality\n>>> and then to use the libified \"git apply\" in run_apply() instead of\n>>> launching a separate \"git apply\" process. One benefit from this is\n>>> that we could probably get rid of the read_cache_from() call at the\n>>> end of run_apply() and this would likely further speed up things. Also\n>>> avoiding to launch separate processes might be a win especially on\n>>> Windows.\n>>\n>> The amount of global variables in apply.c is just scary. Libification\n>> will need some cleanup first (i'm not against it though). Out of\n>> curiosity, how long does it take to do \"git update-index <one modified\n>> path>\" on this repo? That would cover read_cache_from() and\n>> write_cache().\n\nSure I get:\n\n$ time git update-index <README\n\nreal    0m0.079s\nuser    0m0.057s\nsys     0m0.022s\n\n>> While you're measuring, maybe sprinkle some\n>> trace_performance() in apply.c:apply_patch() to get an idea where time\n>> is most spent in?\n\nEach call to read_cache() takes around 0.07 seconds and each call to\ncheck_patch_list() takes around 0.045 seconds, the rest of\napply_patch() is not significant.\n\n> I did some experiment.\n\nThanks for those experiments. They are very interesting!\n\n> The calls from am are basically\n>\n> for i in $PATCHES; do git apply --cached $i; git commit; done\n>\n> and we can approximate the libification version of that with\n>\n> git apply --cached $PATCHES\n>\n> (I hacked git-apply to do write-tree after each patch, close enough to\n> git-commit).\n>\n> I tried it on my shallow-deepen series, 26 patches. The \"for; do\n> git-apply;done\" command took 0m0.482s (real's time), taskset does not\n> affect much for me, while the \"libification\" took just  0m0.105s.\n> That's a very impressive number to aim for, and git.git is a small\n> repo.\n\nYeah, from my tests it also looks like there is a lot that could be\ngained from the libification.\n\nThanks,\nChristian.\n"},{"id":"280058","messageId":"CAP8UFD2=xzn=GJLHjWD0R4TpFb3JauGAHYW0nSb+B4OAoh-0Vg@mail.gmail.com","threadId":"41502","inReplyTo":"CACBZZX7rVAdzfCm=0FdrCXSx8a2=a8n7pjq1ZSW-V3fzmaSGWw@mail.gmail.com","subject":"Re: Rebase performance","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-03-02T10:13:44Z","receivedAt":"2016-03-02T10:13:44Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Feb 25, 2016 at 5:31 PM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Wed, Feb 24, 2016 at 11:09 PM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>\n> [Resent because I was accidentally in GMail's HTML mode and the ML rejected it]\n>\n>> If there was a config option called maybe \"rebase.taskset\" or\n>> \"rebase.setcpuaffinity\" that could be set to ask the OS for all the\n>> rebase child processes to be run on the same core, people who run many\n>> rebases on big repos on big servers as we do at Booking.com could\n>> easily benefit from a nice speed up.\n>>\n>> Technically the option may make git-rebase--am.sh call \"git am\" using\n>> \"taskset\" (if taskset is available on the current OS).\n>\n> I think aside from issues with git-apply this would be an interesting\n> feature to have in git. I.e. some general facility to intercept\n> commands and inject a prefix command in front of them, whether that's\n> taskset, nice/ionice, strace etc.\n\nI started to take a look at that. It could be done in git.c but would\nbe a bit involved as we would have to check the config option or the\nenv variable that describe the prefix command early and exec the\nprefixed command while disabling the config option or the env\nvariable. I wonder if people would prefer an env variable or a config\noption by the way.\n\n>> Another possibility would be to libify the \"git apply\" functionality\n>> and then to use the libified \"git apply\" in run_apply() instead of\n>> launching a separate \"git apply\" process. One benefit from this is\n>> that we could probably get rid of the read_cache_from() call at the\n>> end of run_apply() and this would likely further speed up things. Also\n>> avoiding to launch separate processes might be a win especially on\n>> Windows.\n>\n> Yeah that should help in this particular case and make the taskset\n> redundant since the whole sequence of operations would all be on one\n> core, right?\n\nYeah, all the applying would happen in one process, so hopefully it\nwould not be moved often to another core or cpu.\n\n> At the risk of derailing this thread, a thing that would make rebase\n> even faster I think would be to change it so that instead of applying\n> a patch at a time to the working tree the whole operation takes place\n> on temporary trees & commits and then we'll eventually move the branch\n> pointer to that once it's finished.\n>\n> I.e. there's no reason for why a sequence of 1000 patches where a\n> FOO.txt is changed from \"hi1\", \"hi2\", \"hi3\", ... would be noticeably\n> slower than applying the same changes with git-fast-import.\n>\n> Of course this would require a lot of nuances, e.g. if there's a\n> conflict we'd need to change the working tree & index as we do now\n> before continuing.\n>\n> Has anyone looked into some advanced refactoring of the rebase process\n> that would work like this, or has some feedback on why this would be\n> dumb or that there's a better way to do it?\n\nMy opinion is that such advanced refactoring can happen on top of\nlibifying the \"git apply\" functionality, so I started to work on that.\n"}]}