{"thread":{"id":"38826","subject":"[GSoC] Applying for conversion scripts to builtins","startedAt":"2015-03-16T16:49:59Z","lastAt":"2015-03-17T18:38:39Z","messageCount":6,"participants":["Yurii Shevtsov","Matthieu Moy","Paul Tan","Duy Nguyen","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"257799","messageId":"CAHLaBNJkL1CUJEk=cH=CLcDvZtoAr+PiCo2KHjfMLUKsugtRPA@mail.gmail.com","threadId":"38826","inReplyTo":null,"subject":"[GSoC] Applying for conversion scripts to builtins","fromName":"Yurii Shevtsov","fromEmail":"ungetch@gmail.com","sentAt":"2015-03-16T16:49:59Z","receivedAt":"2015-03-16T16:49:59Z","isPatch":false,"sender":{"key":"ungetch@gmail.com","avatar":null},"body":"I'm going to write for this idea. As I know good proposal should\ncontain timeline and Todo estimations. What should I write in my\nproposal, since there is no clear plan for converting scripts to\nbuiltins. Thanks in advance!\n"},{"id":"257807","messageId":"vpqpp884y6z.fsf@anie.imag.fr","threadId":"38826","inReplyTo":"CAHLaBNJkL1CUJEk=cH=CLcDvZtoAr+PiCo2KHjfMLUKsugtRPA@mail.gmail.com","subject":"Re: [GSoC] Applying for conversion scripts to builtins","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-03-16T18:03:16Z","receivedAt":"2015-03-16T18:03:16Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Yurii Shevtsov writes:\n\n> I'm going to write for this idea. As I know good proposal should\n> contain timeline and Todo estimations. What should I write in my\n> proposal, since there is no clear plan for converting scripts to\n> builtins. Thanks in advance!\n\nThe fact that there is no clear plan is part of the plan. The idea is\nthat how much can be converted depends highly on how the GSoC goes. You\nalready saw with your microproject that something apparently easy can be\nharder than it seems.\n\nSee this thread for a discussion on the topic:\n\n http://thread.gmane.org/gmane.comp.version-control.git/264050/focus=21366\n\nNow, it's up to you to make a good proposal, i.e. both convince people\nthat you can do a good job, and OTOH be realistic about what can be done\nin a GSoC.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"257825","messageId":"CACRoPnQDopKNW2oc=UiLhupBBSh5ZmUHR7hU5aeguD4OVd1uPQ@mail.gmail.com","threadId":"38826","inReplyTo":"CAHLaBNJkL1CUJEk=cH=CLcDvZtoAr+PiCo2KHjfMLUKsugtRPA@mail.gmail.com","subject":"Re: [GSoC] Applying for conversion scripts to builtins","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-03-17T00:22:58Z","receivedAt":"2015-03-17T00:22:58Z","isPatch":false,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Tue, Mar 17, 2015 at 12:49 AM, Yurii Shevtsov wrote:\n> I'm going to write for this idea. As I know good proposal should\n> contain timeline and Todo estimations. What should I write in my\n> proposal, since there is no clear plan for converting scripts to\n> builtins. Thanks in advance!\n\nI'm actually writing a proposal for the same topic because I somehow\nended up with a working prototype of git-pull.c while exploring the\ninternal git API ;). It's not ready as a patch yet though as there are\nsome problems with git's internal API which causes e.g. double free\nerrors and too much code complexity due to required functionality not\nbeing exposed by builtins, which will have to be addressed.\n\nGenerally, it would be easy to convert any shell script to C by just\nusing the run_command* functions (and in less lines of code), but that\nwould not be taking advantage of the potential benefits in porting\nshell scripts to C. To summarize the (ideal) requirements:\n\n* zero spawning of processes so that the internal object/config/index\ncache can be taken advantage of. (and to avoid the process spawning\noverhead which is relative large in e.g. Windows)\n\n* avoid needless parsing since we have direct access to the C data\nstructures.\n\n* use the internal API as much as possible: share code between the\nbuiltins (e.g. fmt-merge-msg.c, exposed in fmt-merge-msg.h) in order\nto reduce code complexity.\n\nThe biggest wins would definitely be portability, but there may be\nperformance improvements, though they are theoretical at this point.\n\nI'm not exactly sure if the above requirements are sane, which is why\nI'm also CC-ing Dscho who knows the problems of git on Windows more\nthan I do.\n\nRegards,\nPaul\n"},{"id":"257826","messageId":"CACsJy8BrdjNO-3ZFMVjbursTfeWjB0CMPN0-GmNRHFTLdqBvhg@mail.gmail.com","threadId":"38826","inReplyTo":"CACRoPnQDopKNW2oc=UiLhupBBSh5ZmUHR7hU5aeguD4OVd1uPQ@mail.gmail.com","subject":"Re: [GSoC] Applying for conversion scripts to builtins","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-17T01:34:48Z","receivedAt":"2015-03-17T01:34:48Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 17, 2015 at 7:22 AM, Paul Tan wrote:\n> Hi,\n>\n> On Tue, Mar 17, 2015 at 12:49 AM, Yurii Shevtsov wrote:\n>> I'm going to write for this idea. As I know good proposal should\n>> contain timeline and Todo estimations. What should I write in my\n>> proposal, since there is no clear plan for converting scripts to\n>> builtins. Thanks in advance!\n>\n> I'm actually writing a proposal for the same topic because I somehow\n> ended up with a working prototype of git-pull.c while exploring the\n> internal git API ;). It's not ready as a patch yet though as there are\n> some problems with git's internal API which causes e.g. double free\n> errors and too much code complexity due to required functionality not\n> being exposed by builtins, which will have to be addressed.\n>\n> Generally, it would be easy to convert any shell script to C by just\n> using the run_command* functions (and in less lines of code), but that\n> would not be taking advantage of the potential benefits in porting\n> shell scripts to C. To summarize the (ideal) requirements:\n\nWhile run_command() is not ideal, it would be a good intermediate\nstate where you can verify with the test suite that the C skeleton\nafter rewrite is working ok. Then you can start killing run_command()\nin subsequent patches. That would be much easier to review code too.\n-- \nDuy\n"},{"id":"257850","messageId":"4355599932558291b22313f244eda9bd@www.dscho.org","threadId":"38826","inReplyTo":"CACRoPnQDopKNW2oc=UiLhupBBSh5ZmUHR7hU5aeguD4OVd1uPQ@mail.gmail.com","subject":"Re: [GSoC] Applying for conversion scripts to builtins","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-03-17T11:56:15Z","receivedAt":"2015-03-17T11:56:15Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-03-17 01:22, Paul Tan wrote:\n\n> On Tue, Mar 17, 2015 at 12:49 AM, Yurii Shevtsov wrote:\n>\n> Generally, it would be easy to convert any shell script to C by just\n> using the run_command* functions (and in less lines of code), but that\n> would not be taking advantage of the potential benefits in porting\n> shell scripts to C. To summarize the (ideal) requirements:\n> \n> * zero spawning of processes so that the internal object/config/index\n> cache can be taken advantage of. (and to avoid the process spawning\n> overhead which is relative large in e.g. Windows)\n\nSpawning definitely uses up many more resources on Windows.\n\nHowever, spawning a full-fledged Bash requires MSys (or soon MSys2) to spin up an entire POSIX emulation layer. This costs us dearly. For example, when I run the t3404 test (which exercises scripting heavily, what with `git rebase -i` being implemented as a shell script) on MacOSX, it takes roughly a minute to complete. On a comparable Windows machine, it takes roughly 12 minutes to complete.\n\nTherefore, I would wager a bet that just the mere conversion of a shell script into even a primitive `run_command()`-based builtin would help performance on Windows in a noticeable manner.\n\nOf course, it would be *even nicer* to avoid the spawning altogether.\n\n> * avoid needless parsing since we have direct access to the C data\n> structures.\n\nTrue that. Turning SHA-1s into strings, spawning, and reparsing the same SHA-1 is quite a lot of unnecessary churn.\n\nThe biggest benefit of avoiding needless parsing, however, is not performance. It is avoiding quoting issues. This is particularly so on Windows, where Git is sometimes called from outside a shell environment, where we have to deal with inconsistent quoting because it is every Windows program's own job to parse the command-line, including the quoting.\n\n> * use the internal API as much as possible: share code between the\n> builtins (e.g. fmt-merge-msg.c, exposed in fmt-merge-msg.h) in order\n> to reduce code complexity.\n\nThat is definitely something that even the Git maintainer should be interested in (he does not touch Windows, therefore the performance differences do not concern him): by sharing code paths between different subcommands, you ensure that you have to fix problems only once, not twice or more.\n\nConcrete example: on Windows, we have file locking issues because files that are in use cannot be deleted. For that reason, we have Windows-specific code that is \"nice\" by trying harder to delete files, giving programs a little time to let their locks go. This locking issue happens also when a virus scanner \"uses\", say, the .git-rewrite/revs file that was written by `git filter-branch`, while said shell script already wants to delete the file because it is obsolete. If `git filter-branch` were a builtin, the bug would already be fixed due to our override of the `unlink()` function in C. Now we have to fix that bug separately because `filter-branch` is a shell script.\n\n> The biggest wins would definitely be portability, but there may be\n> performance improvements, though they are theoretical at this point.\n> \n> I'm not exactly sure if the above requirements are sane, which is why\n> I'm also CC-ing Dscho who knows the problems of git on Windows more\n> than I do.\n\nThanks for bringing this to my attention. I hope I managed to add useful information to the discussion.\n\nCiao,\nJohannes\n"},{"id":"257876","messageId":"xmqqpp87o4eo.fsf@gitster.dls.corp.google.com","threadId":"38826","inReplyTo":"4355599932558291b22313f244eda9bd@www.dscho.org","subject":"Re: [GSoC] Applying for conversion scripts to builtins","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-17T18:38:39Z","receivedAt":"2015-03-17T18:38:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Therefore, I would wager a bet that just the mere conversion of a\n> shell script into even a primitive `run_command()`-based builtin would\n> help performance on Windows in a noticeable manner.\n\nAs you correctly allege, if a patch rewrote a shell-scripted\nporcelain by using series of run_command() and doing nothing else, I\nwould have asked \"is that an improvement?\", without knowing that.\n\n> Of course, it would be *even nicer* to avoid the spawning altogether.\n\nYeah, that, too ;-)\n\n> The biggest benefit of avoiding needless parsing, however, is not\n> performance. It is avoiding quoting issues. This is particularly so on\n> Windows, where Git is sometimes called from outside a shell\n> environment, where we have to deal with inconsistent quoting because\n> it is every Windows program's own job to parse the command-line,\n> including the quoting.\n>\n> Concrete example: on Windows, we have file locking issues because\n> files that are in use cannot be deleted. For that reason, we have\n> Windows-specific code that is \"nice\" by trying harder to delete files,\n> giving programs a little time to let their locks go. This locking\n> issue happens also when a virus scanner \"uses\"...\n\nThese are definitely good advices from the area expert.\n\nThanks for a bunch of good input.\n"}]}