{"thread":{"id":"9934","subject":"[PATCH 0/5] Yet another builtin-fetch round","startedAt":"2007-09-19T04:49:23Z","lastAt":"2007-09-20T05:09:42Z","messageCount":4,"participants":["Shawn O. Pearce","Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"53544","messageId":"20070919044923.GP3099@spearce.org","threadId":"9934","inReplyTo":null,"subject":"[PATCH 0/5] Yet another builtin-fetch round","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-19T04:49:23Z","receivedAt":"2007-09-19T04:49:23Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Another short series for db/fetch-pack, still in pu. Aside from\noptimizing the pipeline on the native transport (so we only invoke\nthe remote process we need once vs. twice) I'm actually now quite\ncomfortable with this whole series and think it is ready for next.\n\nI'm certainly running it in production, and will be until it is\nmerged. The performance difference is too big for me (and at least\nsome of my coworkers) to not be doing so. If there are any specific\nreasons why this topic is not ready for next or is unsuitable for\nmerging please let me know so I can take the time to correct it.\n\n1/5 Rename remote.uri to remote.url within remote handling internals\n2/5 Refactor struct transport_ops inlined into struct transport\n3/5 Always obtain fetch-pack arguments from struct fetch_pack_args\n\n These three are basic code cleanups for small issues that\n bothered me about the original implementation of builtin-fetch.\n Now is just as good of a time as any to cleanup the code and make\n it more maintainable. I think the overall total line count is\n reduced by these three patches.\n\n* Ensure builtin-fetch honors {fetch,transfer}.unpackLimit\n* Fix memory leaks when disconnecting transport instances\n\n Fixes two known (but minor) outstanding bugs. At this point\n I do not know of any other bugs in builtin-fetch so I would\n really appreciate testing reports from other people, especially\n those whose uses cases might stray outside of my workflow. Hah,\n I did not tell you my workflow. ;-)\n\n-- \nShawn.\n"},{"id":"53618","messageId":"Pine.LNX.4.64.0709192233360.21941@iabervon.org","threadId":"9934","inReplyTo":"20070919044923.GP3099@spearce.org","subject":"Re: [PATCH 0/5] Yet another builtin-fetch round","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-09-20T02:40:30Z","receivedAt":"2007-09-20T02:40:30Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 19 Sep 2007, Shawn O. Pearce wrote:\n\n> Another short series for db/fetch-pack, still in pu. Aside from\n> optimizing the pipeline on the native transport (so we only invoke\n> the remote process we need once vs. twice) I'm actually now quite\n> comfortable with this whole series and think it is ready for next.\n\nWhile it's still in pu, should these series of corrections be amended into \nthe original series (for the ones that correct new code)? Most of the \nbefore-fixing states aren't worth saving as project history.\n\n> I'm certainly running it in production, and will be until it is\n> merged. The performance difference is too big for me (and at least\n> some of my coworkers) to not be doing so. If there are any specific\n> reasons why this topic is not ready for next or is unsuitable for\n> merging please let me know so I can take the time to correct it.\n\nGood to hear that it's working for you. It's been working for me since \nJuly, for my usage patterns, but they're not very extensive, and I was \nmostly relying on the tests.\n\n> 1/5 Rename remote.uri to remote.url within remote handling internals\n> 2/5 Refactor struct transport_ops inlined into struct transport\n> 3/5 Always obtain fetch-pack arguments from struct fetch_pack_args\n> \n> These three are basic code cleanups for small issues that\n> bothered me about the original implementation of builtin-fetch.\n> Now is just as good of a time as any to cleanup the code and make\n> it more maintainable. I think the overall total line count is\n> reduced by these three patches.\n\nAll of these look right.\n\n> * Ensure builtin-fetch honors {fetch,transfer}.unpackLimit\n> * Fix memory leaks when disconnecting transport instances\n> \n> Fixes two known (but minor) outstanding bugs. At this point\n> I do not know of any other bugs in builtin-fetch so I would\n> really appreciate testing reports from other people, especially\n> those whose uses cases might stray outside of my workflow. Hah,\n> I did not tell you my workflow. ;-)\n\nLooks good, but I didn't really check over them.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"53621","messageId":"7v6426m110.fsf@gitster.siamese.dyndns.org","threadId":"9934","inReplyTo":"Pine.LNX.4.64.0709192233360.21941@iabervon.org","subject":"Re: [PATCH 0/5] Yet another builtin-fetch round","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-20T04:08:43Z","receivedAt":"2007-09-20T04:08:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> On Wed, 19 Sep 2007, Shawn O. Pearce wrote:\n>\n>> Another short series for db/fetch-pack, still in pu. Aside from\n>> optimizing the pipeline on the native transport (so we only invoke\n>> the remote process we need once vs. twice) I'm actually now quite\n>> comfortable with this whole series and think it is ready for next.\n>\n> While it's still in pu, should these series of corrections be amended into \n> the original series (for the ones that correct new code)? Most of the \n> before-fixing states aren't worth saving as project history.\n\nYeah, I was wondering if that is a sane thing to do. It is\nmerely additional work to arrive at the same tree state, but\nmight be a good investment in the longer term.\n"},{"id":"53625","messageId":"7vsl59ly7d.fsf@gitster.siamese.dyndns.org","threadId":"9934","inReplyTo":"7v6426m110.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] Yet another builtin-fetch round","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-20T05:09:42Z","receivedAt":"2007-09-20T05:09:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Daniel Barkalow writes:\n>\n>> On Wed, 19 Sep 2007, Shawn O. Pearce wrote:\n>>\n>>> Another short series for db/fetch-pack, still in pu. Aside from\n>>> optimizing the pipeline on the native transport (so we only invoke\n>>> the remote process we need once vs. twice) I'm actually now quite\n>>> comfortable with this whole series and think it is ready for next.\n>>\n>> While it's still in pu, should these series of corrections be amended into \n>> the original series (for the ones that correct new code)? Most of the \n>> before-fixing states aren't worth saving as project history.\n>\n> Yeah, I was wondering if that is a sane thing to do. It is\n> merely additional work to arrive at the same tree state, but\n> might be a good investment in the longer term.\n\nHeh, I did not realize that they are now all part of 'next' so\nthat's moot.\n"}]}