{"thread":{"id":"44482","subject":"RFC: Enable delayed responses to Git clean/smudge filter requests","startedAt":"2016-11-14T21:10:12Z","lastAt":"2017-01-11T12:57:58Z","messageCount":16,"participants":["Lars Schneider","Eric Wong","Junio C Hamano","Jakub Narębski","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"305918","messageId":"D10F7C47-14E8-465B-8B7A-A09A1B28A39F@gmail.com","threadId":"44482","inReplyTo":null,"subject":"RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-14T21:09:56Z","receivedAt":"2016-11-14T21:10:12Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi,\n\nGit always performs a clean/smudge filter on files in sequential order.\nSometimes a filter operation can take a noticeable amount of time. \nThis blocks the entire Git process.\n\nI would like to give a filter process the possibility to answer Git with\n\"I got your request, I am processing it, ask me for the result later!\".\n\nI see the following way to realize this:\n\nIn unpack-trees.c:check_updates() [1] we loop through the cache \nentries and \"ask me later\" could be an acceptable return value of the \ncheckout_entry() call. The loop could run until all entries returned\nsuccess or error.\n\nThe filter machinery is triggered in various other places in Git and\nall places that want to support \"ask me later\" would need to be patched \naccordingly.\n\n--\n\nDo you think this could be a viable approach?\nDo you see a better way?\n\nThanks,\nLars\n\n\n[1] https://github.com/git/git/blob/3ab228137f980ff72dbdf5064a877d07bec76df9/unpack-trees.c#L267\n\n"},{"id":"305929","messageId":"20161115010356.GA29602@starla","threadId":"44482","inReplyTo":"D10F7C47-14E8-465B-8B7A-A09A1B28A39F@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-11-15T01:03:56Z","receivedAt":"2016-11-15T01:04:02Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Lars Schneider wrote:\n> Hi,\n> \n> Git always performs a clean/smudge filter on files in sequential order.\n> Sometimes a filter operation can take a noticeable amount of time. \n> This blocks the entire Git process.\n\nI have the same problem in many places which aren't git :>\n\n> I would like to give a filter process the possibility to answer Git with\n> \"I got your request, I am processing it, ask me for the result later!\".\n> \n> I see the following way to realize this:\n> \n> In unpack-trees.c:check_updates() [1] we loop through the cache \n> entries and \"ask me later\" could be an acceptable return value of the \n> checkout_entry() call. The loop could run until all entries returned\n> success or error.\n> \n> The filter machinery is triggered in various other places in Git and\n> all places that want to support \"ask me later\" would need to be patched \n> accordingly.\n\nThat all sounds reasonable.\n\nThe filter itself would need to be aware of parallelism\nif it lives for multiple objects, right?\n\n> Do you think this could be a viable approach?\n\nIt'd probably require a bit of work, but yes, I think it's viable.\n\nWe already do this with curl_multi requests for parallel\nfetching from dumb HTTP servers, but that's driven by curl\ninternals operating with a select/poll loop.\n\nPerhaps the curl API could be a good example for doing this.\n\n> Do you see a better way?\n\nNope, I prefer non-blocking state machines to threads for\ndebuggability and determinism.\n\nAnyways, I'll plan on doing something similar (in Perl) with the\nsynchronous parts of public-inbox which relies on \"cat-file --batch\"\nat some point... (my rotational disks are sloooooooow :<)\n"},{"id":"305950","messageId":"5BC69AC1-5499-4F73-816A-D8786106D796@gmail.com","threadId":"44482","inReplyTo":"20161115010356.GA29602@starla","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-15T14:29:09Z","receivedAt":"2016-11-15T14:29:17Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 15 Nov 2016, at 02:03, Eric Wong wrote:\n> \n> Lars Schneider wrote:\n>> Hi,\n>> \n>> Git always performs a clean/smudge filter on files in sequential order.\n>> Sometimes a filter operation can take a noticeable amount of time. \n>> This blocks the entire Git process.\n> \n> I have the same problem in many places which aren't git :>\n> \n>> I would like to give a filter process the possibility to answer Git with\n>> \"I got your request, I am processing it, ask me for the result later!\".\n>> \n>> I see the following way to realize this:\n>> \n>> In unpack-trees.c:check_updates() [1] we loop through the cache \n>> entries and \"ask me later\" could be an acceptable return value of the \n>> checkout_entry() call. The loop could run until all entries returned\n>> success or error.\n>> \n>> The filter machinery is triggered in various other places in Git and\n>> all places that want to support \"ask me later\" would need to be patched \n>> accordingly.\n> \n> That all sounds reasonable.\n> \n> The filter itself would need to be aware of parallelism\n> if it lives for multiple objects, right?\n\nCorrect. This way Git doesn't need to deal with threading...\n\n\n>> Do you think this could be a viable approach?\n> \n> It'd probably require a bit of work, but yes, I think it's viable.\n> \n> We already do this with curl_multi requests for parallel\n> fetching from dumb HTTP servers, but that's driven by curl\n> internals operating with a select/poll loop.\n> \n> Perhaps the curl API could be a good example for doing this.\n\nThanks for the pointer!\n\n\n>> Do you see a better way?\n> \n> Nope, I prefer non-blocking state machines to threads for\n> debuggability and determinism.\n\nAgreed!\n\n\n> Anyways, I'll plan on doing something similar (in Perl) with the\n> synchronous parts of public-inbox which relies on \"cat-file --batch\"\n> at some point... (my rotational disks are sloooooooow :<)\n\nThat sounds interesting! What changes to you have in mind for \n\"cat-file --batch\"? We are thinking about performance improvements\nin that area, too. I would be happy to help reviewing your patches!\n\nThanks a lot for your RFC feedback,\nLars\n"},{"id":"305972","messageId":"xmqqvavotych.fsf@gitster.mtv.corp.google.com","threadId":"44482","inReplyTo":"5BC69AC1-5499-4F73-816A-D8786106D796@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-15T18:03:10Z","receivedAt":"2016-11-15T18:03:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> The filter itself would need to be aware of parallelism\n>> if it lives for multiple objects, right?\n>\n> Correct. This way Git doesn't need to deal with threading...\n\nI think you need to be careful about three things (at least; there\nmay be more):\n\n * Codepaths that check out multiple cache entries do rely on the\n order of checkout. We checkout removals first to make room so\n that creation of a path X can succeed if an existing path X/Y\n that used to want to see X as a directory can succeed (see the\n use of checkout_entry() by \"git checkout\", which does have two\n separate loops to explicitly guarantee this), for example. I\n think \"remove all and then create\" you do not specifically have\n to worry about with the proposed change, but you may need to\n inspect and verify there aren't other kind of order dependency.\n\n * Done naively, it will lead to unmaintainable code, like this:\n\n + struct list_of_cache_entries *list = ...;\n for (i = 0; i < active_nr; i++)\n - checkout_entry(active_cache[i], state, NULL);\n + if (checkout_entry(active_cache[i], state, NULL) == DELAYED)\n + add_cache_to_queue(&list, active_cache[i]);\n + while (list) {\n + wait_for_checkout_to_finish(*list);\n + list = list->next;\n + }\n\n I do not think we want to see such a rewrite all over the\n codepaths. It might be OK to add such a \"these entries are known\n to be delayed\" list in struct checkout so that the above becomes\n more like this:\n\n for (i = 0; i < active_nr; i++)\n checkout_entry(active_cache[i], state, NULL);\n + checkout_entry_finish(state);\n\n That is, addition of a single \"some of the checkout_entry() calls\n done so far might have been lazy, and I'll give them a chance to\n clean up\" might be palatable. Anything more than that on the\n caller side is not.\n\n * You'd need to rein in the maximum parallelism somehow, as you do\n not want to see hundreds of competing filter processes starting\n only to tell the main loop over an index with hundreds of entries\n that they are delayed checkouts.\n\n"},{"id":"305974","messageId":"20161115182717.GA5162@starla","threadId":"44482","inReplyTo":"5BC69AC1-5499-4F73-816A-D8786106D796@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-11-15T18:27:17Z","receivedAt":"2016-11-15T18:27:34Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Lars Schneider wrote:\n> > On 15 Nov 2016, at 02:03, Eric Wong wrote:\n> \n> > Anyways, I'll plan on doing something similar (in Perl) with the\n> > synchronous parts of public-inbox which relies on \"cat-file --batch\"\n> > at some point... (my rotational disks are sloooooooow :<)\n> \n> That sounds interesting! What changes to you have in mind for \n> \"cat-file --batch\"? We are thinking about performance improvements\n> in that area, too. I would be happy to help reviewing your patches!\n\nI'm not touching \"cat-file --batch\" itself, just the Perl code\nthat uses it. (still trying my best to avoid working on C or\nany compiled languages);\n\nThere'll probably be a Perl queue mechanism so there's still\nonly one cat-file process running per-inbox to avoid overloading\non seeks; but the Perl daemons should be able to handle network\nduties and other in-memory operations while it waits for\ncat-file.\n"},{"id":"306040","messageId":"17709AFF-3C2D-4EC0-97DC-BD750F514D0B@gmail.com","threadId":"44482","inReplyTo":"xmqqvavotych.fsf@gitster.mtv.corp.google.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-16T09:53:17Z","receivedAt":"2016-11-16T09:53:26Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Nov 2016, at 19:03, Junio C Hamano wrote:\n\n> Lars Schneider writes:\n> \n>>> The filter itself would need to be aware of parallelism\n>>> if it lives for multiple objects, right?\n>> \n>> Correct. This way Git doesn't need to deal with threading...\n> \n> I think you need to be careful about three things (at least; there\n> may be more):\n> \n> * Codepaths that check out multiple cache entries do rely on the\n> order of checkout. We checkout removals first to make room so\n> that creation of a path X can succeed if an existing path X/Y\n> that used to want to see X as a directory can succeed (see the\n> use of checkout_entry() by \"git checkout\", which does have two\n> separate loops to explicitly guarantee this), for example. I\n> think \"remove all and then create\" you do not specifically have\n> to worry about with the proposed change, but you may need to\n> inspect and verify there aren't other kind of order dependency.\n\nOK\n\n\n> * Done naively, it will lead to unmaintainable code, like this:\n> \n> + struct list_of_cache_entries *list = ...;\n> for (i = 0; i < active_nr; i++)\n> - checkout_entry(active_cache[i], state, NULL);\n> + if (checkout_entry(active_cache[i], state, NULL) == DELAYED)\n> + add_cache_to_queue(&list, active_cache[i]);\n> + while (list) {\n> + wait_for_checkout_to_finish(*list);\n> + list = list->next;\n> + }\n> \n> I do not think we want to see such a rewrite all over the\n> codepaths. It might be OK to add such a \"these entries are known\n> to be delayed\" list in struct checkout so that the above becomes\n> more like this:\n> \n> for (i = 0; i < active_nr; i++)\n> checkout_entry(active_cache[i], state, NULL);\n> + checkout_entry_finish(state);\n> \n> That is, addition of a single \"some of the checkout_entry() calls\n> done so far might have been lazy, and I'll give them a chance to\n> clean up\" might be palatable. Anything more than that on the\n> caller side is not.\n\nI haven't thought hard about the implementation, yet, but I'll try \nto stick to your suggestion and change as less code as possible on \nthe caller sides.\n\n\n> * You'd need to rein in the maximum parallelism somehow, as you do\n> not want to see hundreds of competing filter processes starting\n> only to tell the main loop over an index with hundreds of entries\n> that they are delayed checkouts.\n\nI intend to implement this feature only for the new long running filter\nprocess protocol. OK with you?\n\n\nThanks,\nLars\n"},{"id":"306066","messageId":"xmqqpolvqoka.fsf@gitster.mtv.corp.google.com","threadId":"44482","inReplyTo":"17709AFF-3C2D-4EC0-97DC-BD750F514D0B@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T18:15:01Z","receivedAt":"2016-11-16T18:15:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> * You'd need to rein in the maximum parallelism somehow, as you do\n>> not want to see hundreds of competing filter processes starting\n>> only to tell the main loop over an index with hundreds of entries\n>> that they are delayed checkouts.\n>\n> I intend to implement this feature only for the new long running filter\n> process protocol. OK with you?\n\nDo you mean that a long-running filter process interacting with\nconvert_to_worktree() called from checkout_entry() will be the only\ncodepath that will spawn multiple processes or threads? \n\nThat is fine, but it does not change the fact that you still need to\nlimit the maximum parallelism there.\n"},{"id":"306072","messageId":"2F93C9B4-157C-4F5C-9BD5-A67AA519757A@gmail.com","threadId":"44482","inReplyTo":"xmqqpolvqoka.fsf@gitster.mtv.corp.google.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-16T18:47:17Z","receivedAt":"2016-11-16T18:47:25Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 16 Nov 2016, at 19:15, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>>> * You'd need to rein in the maximum parallelism somehow, as you do\n>>> not want to see hundreds of competing filter processes starting\n>>> only to tell the main loop over an index with hundreds of entries\n>>> that they are delayed checkouts.\n>> \n>> I intend to implement this feature only for the new long running filter\n>> process protocol. OK with you?\n> \n> Do you mean that a long-running filter process interacting with\n> convert_to_worktree() called from checkout_entry() will be the only\n> codepath that will spawn multiple processes or threads? \n> \n> That is fine, but it does not change the fact that you still need to\n> limit the maximum parallelism there.\n\nFilters using the long running protocol are spawned only once by Git. \nThe filter process would get all the smudge requests via the pipe\nprotocol and is supposed to manage the parallelism on its own.\n\n- Lars\n"},{"id":"306077","messageId":"xmqqk2c3p70a.fsf@gitster.mtv.corp.google.com","threadId":"44482","inReplyTo":"2F93C9B4-157C-4F5C-9BD5-A67AA519757A@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T19:19:33Z","receivedAt":"2016-11-16T19:19:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> On 16 Nov 2016, at 19:15, Junio C Hamano wrote:\n>> \n>> Lars Schneider writes:\n>> \n>>>> * You'd need to rein in the maximum parallelism somehow, as you do\n>>>> not want to see hundreds of competing filter processes starting\n>>>> only to tell the main loop over an index with hundreds of entries\n>>>> that they are delayed checkouts.\n>>> \n>>> I intend to implement this feature only for the new long running filter\n>>> process protocol. OK with you?\n>> \n>> Do you mean that a long-running filter process interacting with\n>> convert_to_worktree() called from checkout_entry() will be the only\n>> codepath that will spawn multiple processes or threads? \n>> \n>> That is fine, but it does not change the fact that you still need to\n>> limit the maximum parallelism there.\n>\n> Filters using the long running protocol are spawned only once by Git. \n> The filter process would get all the smudge requests via the pipe\n> protocol and is supposed to manage the parallelism on its own.\n\nYes, I think we are on the same page. You need to be careful not to\nlet the filter process go berserk spawning too many threads or\nprocesses.\n"},{"id":"306085","messageId":"5eb682e8-13cb-67f2-a8a9-ec1fa1d139c6@gmail.com","threadId":"44482","inReplyTo":"17709AFF-3C2D-4EC0-97DC-BD750F514D0B@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-11-16T22:41:57Z","receivedAt":"2016-11-16T22:42:19Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 16.11.2016 o 10:53, Lars Schneider pisze:\n> On 15 Nov 2016, at 19:03, Junio C Hamano wrote:\n>> Lars Schneider writes:\n>>\n>>>> The filter itself would need to be aware of parallelism\n>>>> if it lives for multiple objects, right?\n>>>\n>>> Correct. This way Git doesn't need to deal with threading...\n[...]\n\n>> * You'd need to rein in the maximum parallelism somehow, as you do\n>> not want to see hundreds of competing filter processes starting\n>> only to tell the main loop over an index with hundreds of entries\n>> that they are delayed checkouts.\n> \n> I intend to implement this feature only for the new long running filter\n> process protocol. OK with you?\n\nIf I remember and understand it correctly, current version of long\nrunning process protocol processes files sequentially, one by one:\ngit sends file to filter wholly, and receives response wholly.\n\nIn the single-file filter case, git calls filter process as async\ntask, in a separate thread, so that one thread feeds the filter,\nand main thread (I think?) reads from it, to avoid deadlocks.\n\nCouldn't something like this be done for long running filter process,\nvia protocol extension? Namely, Git would send in an async task\ncontent to be filtered, and filter process would stream the response\nback, in any order. If it would be not enough, we could borrow\nidea of channels, and be sending few files back concurrently in\nparallel, as separate channels... though that would probably\nrequire quite a bit of change in caller.\n\nBest,\n-- \nJakub Narębski\n\n"},{"id":"306090","messageId":"xmqq37irouni.fsf@gitster.mtv.corp.google.com","threadId":"44482","inReplyTo":"5eb682e8-13cb-67f2-a8a9-ec1fa1d139c6@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T23:46:25Z","receivedAt":"2016-11-16T23:46:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n>> I intend to implement this feature only for the new long running filter\n>> process protocol. OK with you?\n>\n> If I remember and understand it correctly, current version of long\n> running process protocol processes files sequentially, one by one:\n> git sends file to filter wholly, and receives response wholly.\n>\n> In the single-file filter case, git calls filter process as async\n> task, in a separate thread, so that one thread feeds the filter,\n> and main thread (I think?) reads from it, to avoid deadlocks.\n>\n> Couldn't something like this be done for long running filter process,\n> via protocol extension?\n\nMy reading of the message you are responding to is that Lars means\ndoing so by \"implement this feature\". Instead of returning the\nfiltered bytes, a new protocol lets his filter to say \"No result yet\nfor you to process, ask me later\".\n\n"},{"id":"306100","messageId":"EBC181CD-611D-4FD6-9B84-F0AE9A970745@gmail.com","threadId":"44482","inReplyTo":"xmqq37irouni.fsf@gitster.mtv.corp.google.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-17T09:19:54Z","receivedAt":"2016-11-17T09:20:04Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 17 Nov 2016, at 00:46, Junio C Hamano wrote:\n> \n> Jakub Narębski writes:\n> \n>>> I intend to implement this feature only for the new long running filter\n>>> process protocol. OK with you?\n>> \n>> If I remember and understand it correctly, current version of long\n>> running process protocol processes files sequentially, one by one:\n>> git sends file to filter wholly, and receives response wholly.\n>> \n>> In the single-file filter case, git calls filter process as async\n>> task, in a separate thread, so that one thread feeds the filter,\n>> and main thread (I think?) reads from it, to avoid deadlocks.\n>> \n>> Couldn't something like this be done for long running filter process,\n>> via protocol extension?\n> \n> My reading of the message you are responding to is that Lars means\n> doing so by \"implement this feature\". Instead of returning the\n> filtered bytes, a new protocol lets his filter to say \"No result yet\n> for you to process, ask me later\".\n\nCorrect!\n\n"},{"id":"306405","messageId":"249EE7A4-F297-4537-92A9-0EF75A3B1AEE@gmail.com","threadId":"44482","inReplyTo":"xmqqvavotych.fsf@gitster.mtv.corp.google.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-24T15:45:45Z","receivedAt":"2016-11-24T16:11:01Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 15 Nov 2016, at 19:03, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>>> The filter itself would need to be aware of parallelism\n>>> if it lives for multiple objects, right?\n>> \n>> Correct. This way Git doesn't need to deal with threading...\n> \n> I think you need to be careful about three things (at least; there\n> may be more):\n> \n> ...\n> \n> * Done naively, it will lead to unmaintainable code, like this:\n> \n> ...\n\nHi Junio,\n\nI started to work on the \"delayed responses to Git clean/smudge filter\nrequests\" implementation and I am looking for a recommendation regarding \ncode maintainability:\n\nDeep down in convert.c:636 `apply_multi_file_filter()` [1] the filter learns\nfrom the external process that the filter response is not yet available.\nI need to transport this information quite a few levels up the call\nstack. \n\n\n# Option 1\nI could do this by explicitly passing a pointer such as \"int *is_delayed\" \nto the function. This would mean I need to update the function definitions \nfor all functions on my way through the stack:\n\nint apply_multi_file_filter()\nint apply_filter()\nint convert_to_working_tree_internal()\nint convert_to_working_tree()\n...\n\n# Option 2\nAll these functions pass-through an \"int\" return value that communicates\nif the filter succeeded or failed. I could define a special return value\nto communicate the third state: delayed. \n\n\nWhat way do you think is better from a maintenance point of view?\nI prefer option 2 but I fear that these \"special\" values could confuse\nfuture readers of the code.\n\nThanks,\nLars\n\n\n[1] https://github.com/git/git/blob/e2b2d6a172b76d44cb7b1ddb12ea5bfac9613a44/convert.c#L673-L777\n\n"},{"id":"306520","messageId":"xmqqk2bni8ce.fsf@gitster.mtv.corp.google.com","threadId":"44482","inReplyTo":"249EE7A4-F297-4537-92A9-0EF75A3B1AEE@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-28T21:48:49Z","receivedAt":"2016-11-28T21:49:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> What way do you think is better from a maintenance point of view?\n> I prefer option 2 but I fear that these \"special\" values could confuse\n> future readers of the code.\n\nI recall getting confused by the redefinition of the meaning of\nreturn value from the grep_directory() function when we started\nthreading the working-tree grep codepath at around 5b594f457a;\ncompared to that, as long as the \"special\" (and \"normal\") values are\nmade symbolic constants, I do not think it is too bad.\n"},{"id":"309067","messageId":"CAGZ79kYDPLDU5Dg_CTnpEX+D9bs6BUSSNTHkqpW2nY-b=e9+SQ@mail.gmail.com","threadId":"44482","inReplyTo":"D10F7C47-14E8-465B-8B7A-A09A1B28A39F@gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-09T20:44:37Z","receivedAt":"2017-01-09T20:44:46Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Nov 14, 2016 at 1:09 PM, Lars Schneider\n wrote:\n> Hi,\n>\n> Git always performs a clean/smudge filter on files in sequential order.\n> Sometimes a filter operation can take a noticeable amount of time.\n> This blocks the entire Git process.\n>\n> I would like to give a filter process the possibility to answer Git with\n> \"I got your request, I am processing it, ask me for the result later!\".\n>\n> I see the following way to realize this:\n>\n> In unpack-trees.c:check_updates() [1] we loop through the cache\n> entries and \"ask me later\" could be an acceptable return value of the\n> checkout_entry() call. The loop could run until all entries returned\n> success or error.\n\nLate to this thread, but here is an answer nevertheless.\n\nI am currently working on getting submodules working\nfor working tree modifying commands (prominently checkout, but\nalso read-tree -u and any other caller that uses the code in\nunpack-trees.)\n\nOnce the submodules are supported and used, I anticipate that\nputting the files in the working tree on disk will become a bottle neck,\ni.e. the checkout taking way too long for an oversized project.\n\nSo in the future we have to do something to make checkout fast\nagain, which IMHO is threading. My current vision is to have checkout\nautomatically choose a number of threads based on expected workload,\nc.f. preload-index.c, line 18-25.\n\n> The filter machinery is triggered in various other places in Git and\n> all places that want to support \"ask me later\" would need to be patched\n> accordingly.\n\nI think this makes sense, even in a threaded git-checkout.\nI assume this idea is implemented before threading hits checkout,\nso a question on the design:\n\nWho determines the workload that is acceptable?\nFrom reading this email, it seems to be solely the filter that uses\nas many threads/processes as it thinks is ok.\n\nWould it be possible to enhance the protocol further to have\nGit also mingle with the workload, i.e. tell the filter it is\nallowed to use up (N-M) threads, as it itself already uses\nM out of N configured threads?\n\n(I do not want to discuss the details here, but only if such a thing\nis viable with this approach as well)\n\nThanks,\nStefan\n"},{"id":"309211","messageId":"B46B8E63-4447-4182-B95C-8D1E02D71F6F@gmail.com","threadId":"44482","inReplyTo":"CAGZ79kYDPLDU5Dg_CTnpEX+D9bs6BUSSNTHkqpW2nY-b=e9+SQ@mail.gmail.com","subject":"Re: RFC: Enable delayed responses to Git clean/smudge filter requests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-01-11T12:57:49Z","receivedAt":"2017-01-11T12:57:58Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 09 Jan 2017, at 21:44, Stefan Beller wrote:\n> \n> On Mon, Nov 14, 2016 at 1:09 PM, Lars Schneider\n> wrote:\n>> Hi,\n>> \n>> Git always performs a clean/smudge filter on files in sequential order.\n>> Sometimes a filter operation can take a noticeable amount of time.\n>> This blocks the entire Git process.\n>> \n>> I would like to give a filter process the possibility to answer Git with\n>> \"I got your request, I am processing it, ask me for the result later!\".\n>> \n>> I see the following way to realize this:\n>> \n>> In unpack-trees.c:check_updates() [1] we loop through the cache\n>> entries and \"ask me later\" could be an acceptable return value of the\n>> checkout_entry() call. The loop could run until all entries returned\n>> success or error.\n> \n> Late to this thread, but here is an answer nevertheless.\n> \n> I am currently working on getting submodules working\n> for working tree modifying commands (prominently checkout, but\n> also read-tree -u and any other caller that uses the code in\n> unpack-trees.)\n> \n> Once the submodules are supported and used, I anticipate that\n> putting the files in the working tree on disk will become a bottle neck,\n> i.e. the checkout taking way too long for an oversized project.\n> \n> So in the future we have to do something to make checkout fast\n> again, which IMHO is threading. My current vision is to have checkout\n> automatically choose a number of threads based on expected workload,\n> c.f. preload-index.c, line 18-25.\n\nThat sounds interesting! We are using \"submodule.fetchjobs=0\" to process\nsubmodules in parallel already and it works great! Thanks a lot for\nimplementing this!\n\n\n>> The filter machinery is triggered in various other places in Git and\n>> all places that want to support \"ask me later\" would need to be patched\n>> accordingly.\n> \n> I think this makes sense, even in a threaded git-checkout.\n> I assume this idea is implemented before threading hits checkout,\n> so a question on the design:\n> \n> Who determines the workload that is acceptable?\n> From reading this email, it seems to be solely the filter that uses\n> as many threads/processes as it thinks is ok.\n\nCorrect.\n\n\n> Would it be possible to enhance the protocol further to have\n> Git also mingle with the workload, i.e. tell the filter it is\n> allowed to use up (N-M) threads, as it itself already uses\n> M out of N configured threads?\n> \n> (I do not want to discuss the details here, but only if such a thing\n> is viable with this approach as well)\n\nYes, I think we could give a filter these kind of hints. However, it\nwould, of course, be up to the filter implementation to follow the hints.\n\nIn case you curious, here is the discussion on v1 of the delay implementation:\nhttp://public-inbox.org/git/20170108191736.47359-1-larsxschneider@gmail.com/\n\n\nCheers,\nLars"}]}