{"thread":{"id":"20427","subject":"[PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","startedAt":"2009-08-06T09:55:21Z","lastAt":"2009-08-09T13:42:12Z","messageCount":34,"participants":["Nick Edelen","Johannes Schindelin","Michael J Gruber","Shawn O. Pearce","A Large Angry SCM","Sam Vilain","Nicolas Pitre","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"119764","messageId":"op.ux8i6hrbtdk399@sirnot.private","threadId":"20427","inReplyTo":null,"subject":"[PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-06T09:55:21Z","receivedAt":"2009-08-06T09:55:21Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"SUGGESTED FOR 'PU':\n\nTraversing objects is currently very costly, as every commit and tree must be \nloaded and parsed. Much time and energy could be saved by caching metadata and \ntopological info in an efficient, easily accessible manner. Furthermore, this \ncould improve git's interfacing potential, by providing a condensed summary of \na repository's commit tree.\n\nThis is a series to implement such a revision caching mechanism, aptly named \nrev-cache. The series will provide:\n - a core API to manipulate and traverse caches\n - an integration into the internal revision walker\n - a porcelain front-end providing access to users and (shell) applications\n - a series of tests to verify/demonstrate correctness\n - documentation of the API, porcelain and core concepts\n\nIn cold starts rev-cache has sped up packing and walking by a factor of 4, and \nover twice that on warm starts. Some times on slax for the linux repository:\n\nrev-list --all --objects >/dev/null\n default\n cold 1:13\n warm 0:43\n rev-cache'd\n cold 0:19\n warm 0:02\n\npack-objects --revs --all --stdout >/dev/null\n default\n cold 2:44\n warm 1:21\n rev-cache'd\n cold 0:44\n warm 0:10\n\nThe mechanism is minimally intrusive: most of the changes take place in \nseperate files, and only a handful of git's existing functions are modified.\n\nHope you find this useful.\n\n - Nick\n\n Documentation/rev-cache.txt | 51 +\n Documentation/technical/rev-cache.txt | 336 ++++++\n Makefile | 2 +\n blob.c | 1 +\n blob.h | 1 +\n builtin-rev-cache.c | 284 +++++\n builtin.h | 1 +\n commit.c | 3 +\n commit.h | 2 +\n git.c | 1 +\n list-objects.c | 49 +-\n rev-cache.c | 1832 +++++++++++++++++++++++++++++++++\n revision.c | 89 ++-\n revision.h | 46 +-\n t/t6015-rev-cache-list.sh | 228 ++++\n t/t6015-sha1-dump-diff.py | 36 +\n 16 files changed, 2937 insertions(+), 25 deletions(-)\n create mode 100755 Documentation/rev-cache.txt\n create mode 100755 Documentation/technical/rev-cache.txt\n create mode 100755 builtin-rev-cache.c\n create mode 100755 rev-cache.c\n create mode 100755 t/t6015-rev-cache-list.sh\n create mode 100755 t/t6015-sha1-dump-diff.py\n"},{"id":"119782","messageId":"alpine.DEB.1.00.0908061645470.8306@pacific.mpi-cbg.de","threadId":"20427","inReplyTo":"op.ux8i6hrbtdk399@sirnot.private","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-06T14:48:27Z","receivedAt":"2009-08-06T14:48:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 6 Aug 2009, Nick Edelen wrote:\n\n> SUGGESTED FOR 'PU':\n> \n> Traversing objects is currently very costly, as every commit and tree must be \n> loaded and parsed. Much time and energy could be saved by caching metadata and \n> topological info in an efficient, easily accessible manner. Furthermore, this \n> could improve git's interfacing potential, by providing a condensed summary of \n> a repository's commit tree.\n> \n> This is a series to implement such a revision caching mechanism, aptly named \n> rev-cache. The series will provide:\n> - a core API to manipulate and traverse caches\n> - an integration into the internal revision walker\n> - a porcelain front-end providing access to users and (shell) applications\n> - a series of tests to verify/demonstrate correctness\n> - documentation of the API, porcelain and core concepts\n> \n> In cold starts rev-cache has sped up packing and walking by a factor of 4, and \n> over twice that on warm starts. Some times on slax for the linux repository:\n> \n> rev-list --all --objects >/dev/null\n> default\n> cold 1:13\n> warm 0:43\n> rev-cache'd\n> cold 0:19\n> warm 0:02\n> \n> pack-objects --revs --all --stdout >/dev/null\n> default\n> cold 2:44\n> warm 1:21\n> rev-cache'd\n> cold 0:44\n> warm 0:10\n\nNice!\n\n> The mechanism is minimally intrusive: most of the changes take place in \n> seperate files, and only a handful of git's existing functions are \n> modified.\n\nSorry, I forgot the details, could you quickly remind me why these caches \nare not in the pack index files?\n\n> Documentation/rev-cache.txt | 51 +\n> Documentation/technical/rev-cache.txt | 336 ++++++\n> Makefile | 2 +\n> blob.c | 1 +\n> blob.h | 1 +\n> builtin-rev-cache.c | 284 +++++\n> builtin.h | 1 +\n> commit.c | 3 +\n> commit.h | 2 +\n> git.c | 1 +\n> list-objects.c | 49 +-\n> rev-cache.c | 1832 +++++++++++++++++++++++++++++++++\n> revision.c | 89 ++-\n> revision.h | 46 +-\n> t/t6015-rev-cache-list.sh | 228 ++++\n> t/t6015-sha1-dump-diff.py | 36 +\n\nHmpf.\n\nWe got rid of the last Python script in Git a long time ago, but now two \ndifferent patch series try to sneak that dependency (at least for testing) \nback in.\n\nThat's all the worse because we cannot use Python in msysGit, and Windows \nshould be a platform benefitting dramatically from your work.\n\nCiao,\nDscho\n"},{"id":"119785","messageId":"4A7AEFA8.5010001@drmicha.warpmail.net","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908061645470.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-08-06T14:58:48Z","receivedAt":"2009-08-06T14:58:48Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Johannes Schindelin venit, vidit, dixit 06.08.2009 16:48:\n> Hi,\n> \n> On Thu, 6 Aug 2009, Nick Edelen wrote:\n> \n>> SUGGESTED FOR 'PU':\n>>\n>> Traversing objects is currently very costly, as every commit and tree must be \n>> loaded and parsed. Much time and energy could be saved by caching metadata and \n>> topological info in an efficient, easily accessible manner. Furthermore, this \n>> could improve git's interfacing potential, by providing a condensed summary of \n>> a repository's commit tree.\n>>\n>> This is a series to implement such a revision caching mechanism, aptly named \n>> rev-cache. The series will provide:\n>> - a core API to manipulate and traverse caches\n>> - an integration into the internal revision walker\n>> - a porcelain front-end providing access to users and (shell) applications\n>> - a series of tests to verify/demonstrate correctness\n>> - documentation of the API, porcelain and core concepts\n>>\n>> In cold starts rev-cache has sped up packing and walking by a factor of 4, and \n>> over twice that on warm starts. Some times on slax for the linux repository:\n>>\n>> rev-list --all --objects >/dev/null\n>> default\n>> cold 1:13\n>> warm 0:43\n>> rev-cache'd\n>> cold 0:19\n>> warm 0:02\n>>\n>> pack-objects --revs --all --stdout >/dev/null\n>> default\n>> cold 2:44\n>> warm 1:21\n>> rev-cache'd\n>> cold 0:44\n>> warm 0:10\n> \n> Nice!\n> \n>> The mechanism is minimally intrusive: most of the changes take place in \n>> seperate files, and only a handful of git's existing functions are \n>> modified.\n> \n> Sorry, I forgot the details, could you quickly remind me why these caches \n> are not in the pack index files?\n> \n>> Documentation/rev-cache.txt | 51 +\n>> Documentation/technical/rev-cache.txt | 336 ++++++\n>> Makefile | 2 +\n>> blob.c | 1 +\n>> blob.h | 1 +\n>> builtin-rev-cache.c | 284 +++++\n>> builtin.h | 1 +\n>> commit.c | 3 +\n>> commit.h | 2 +\n>> git.c | 1 +\n>> list-objects.c | 49 +-\n>> rev-cache.c | 1832 +++++++++++++++++++++++++++++++++\n>> revision.c | 89 ++-\n>> revision.h | 46 +-\n>> t/t6015-rev-cache-list.sh | 228 ++++\n>> t/t6015-sha1-dump-diff.py | 36 +\n> \n> Hmpf.\n> \n> We got rid of the last Python script in Git a long time ago, but now two \n> different patch series try to sneak that dependency (at least for testing) \n> back in.\n> \n> That's all the worse because we cannot use Python in msysGit, and Windows \n> should be a platform benefitting dramatically from your work.\n\nIn fact, the test the script performs could be easily rephrased with\n\"sort\", \"uniq\" and \"comm\".\nOTOH: If the walker is supposed to return the exact same orderd list of\ncommits you can just use test_cmp.\n\nMichael\n"},{"id":"119807","messageId":"c77435a80908061039p30b83511qb7c378cfd68a6cf6@mail.gmail.com","threadId":"20427","inReplyTo":"4A7AEFA8.5010001@drmicha.warpmail.net","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-06T17:39:58Z","receivedAt":"2009-08-06T17:39:58Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"Hi there,\n\n> Sorry, I forgot the details, could you quickly remind me why these caches\n> are not in the pack index files?\n\nEr, I'm not sure what you mean. Are you asking why these revision\ncaches are required if we have a pack index, or why they aren't in the\npack index, or something different? I'm thinking probably the second:\nthe short answer is that cache slices are totally independant of pack\nfiles.\n\nIt might be possible to somehow merge revision cache slices with pack\nindexes, but I don't think it'd be a very suitable modification. The\nrev-cache slices are meant to act almost like topo-relation pack\nfiles: new slices are simply new files, seperate slice files can be\nfused (\"repacked\") into a larger one, the index is a (recreatable)\nsingle file associating file (positions) with objects. The format was\ngeared to reducing potential cache/data loss and preventing overly\nlarge cache slices.\n\n>> Hmpf.\n>>\n>> We got rid of the last Python script in Git a long time ago, but now two\n>> different patch series try to sneak that dependency (at least for testing)\n>> back in.\n>>\n>> That's all the worse because we cannot use Python in msysGit, and Windows\n>> should be a platform benefitting dramatically from your work.\n>\n> In fact, the test the script performs could be easily rephrased with\n> \"sort\", \"uniq\" and \"comm\".\n> OTOH: If the walker is supposed to return the exact same orderd list of\n> commits you can just use test_cmp.\n\nThe language that script is written in isn't important. I originally\nwrote it in python because I wanted something quick and wasn't much of\na sh guru (sorry :-/ ). As Micheal said I've no doubt it can easily\nbe converted to shell script -- in fact, I'll try to get a shell\nversion working today.\n\n - Nick\n"},{"id":"119815","messageId":"alpine.DEB.1.00.0908062030340.8306@pacific.mpi-cbg.de","threadId":"20427","inReplyTo":"c77435a80908061039p30b83511qb7c378cfd68a6cf6@mail.gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-06T19:06:59Z","receivedAt":"2009-08-06T19:06:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 6 Aug 2009, Nick Edelen wrote:\n\n> > Sorry, I forgot the details, could you quickly remind me why these \n> > caches are not in the pack index files?\n> \n> Er, I'm not sure what you mean. Are you asking why these revision \n> caches are required if we have a pack index, or why they aren't in the \n> pack index, or something different? I'm thinking probably the second:\n\nYep.\n\n> the short answer is that cache slices are totally independant of pack \n> files.\n\nMy idea with that was that you already have a SHA-1 map in the pack index, \nand if all you want to be able to accelerate the revision walker, you'd \nprobably need something that adds yet another mapping, from commit to \nparents and tree, and from tree to sub-tree and blob (so you can avoid \nunpacking commit and tree objects).\n\nI just thought that it could be more efficient to do it at the time the \npack index is written _anyway_, as nothing will change in the pack after \nthat anyway.\n\nBut I guess I can answer my question easily myself: the boundary commits \nwill not be handled that way.\n\nStill, there is some redundancy between the pack index and your cache, as \nyou have to write out the whole list of SHA-1s all over again. I guess it \nis time to look at the code instead of asking stupid questions.\n\n> It might be possible to somehow merge revision cache slices with pack \n> indexes, but I don't think it'd be a very suitable modification. The \n> rev-cache slices are meant to act almost like topo-relation pack files: \n> new slices are simply new files, seperate slice files can be fused \n> (\"repacked\") into a larger one, the index is a (recreatable) single file \n> associating file (positions) with objects. The format was geared to \n> reducing potential cache/data loss and preventing overly large cache \n> slices.\n> \n> >> Hmpf.\n> >>\n> >> We got rid of the last Python script in Git a long time ago, but now \n> >> two different patch series try to sneak that dependency (at least for \n> >> testing) back in.\n> >>\n> >> That's all the worse because we cannot use Python in msysGit, and \n> >> Windows should be a platform benefitting dramatically from your work.\n> >\n> > In fact, the test the script performs could be easily rephrased with \n> > \"sort\", \"uniq\" and \"comm\". OTOH: If the walker is supposed to return \n> > the exact same orderd list of commits you can just use test_cmp.\n> \n> The language that script is written in isn't important. I originally\n> wrote it in python because I wanted something quick and wasn't much of\n> a sh guru (sorry :-/ ). As Micheal said I've no doubt it can easily\n> be converted to shell script\n\nThat is not what I wanted to hear.\n\n> -- in fact, I'll try to get a shell version working today.\n\nThat is.\n\nThanks,\nDscho\n"},{"id":"119826","messageId":"c77435a80908061301n5e855aeci16af392ed3128651@mail.gmail.com","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908062030340.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-06T20:01:39Z","receivedAt":"2009-08-06T20:01:39Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"Hi,\n\n>My idea with that was that you already have a SHA-1 map in the pack index,\n>and if all you want to be able to accelerate the revision walker, you'd\n>probably need something that adds yet another mapping, from commit to\n>parents and tree, and from tree to sub-tree and blob (so you can avoid\n>unpacking commit and tree objects).\n\nAs I mention in one of the patch descriptions, along with each commit\na list of objects introduced per commit is cached, so no extra I/O is\nnecessary for tree recursion, etc. during traversal.\n\n>I just thought that it could be more efficient to do it at the time the\n>pack index is written _anyway_, as nothing will change in the pack after\n>that anyway.\n\nNothing might change in the pack, but the slices were made to allow\nfor continual addition and refinement of the cache. In a typical\nusage slices will be added and fused on a regular basis, which would\nrequire tinkering in pack indexes if they were combined.\n\n>But I guess I can answer my question easily myself: the boundary commits\n>will not be handled that way.\n>\n>Still, there is some redundancy between the pack index and your cache, as\n>you have to write out the whole list of SHA-1s all over again. I guess it\n>is time to look at the code instead of asking stupid questions.\n\nThe whole revision cache is redundant, technically speaking: nothing\nin it can't be found by rummaging through packs or objects. The point\nof it was to distill out important information for fast, easy access\nof the commit tree.\n\nOn another note, I've eliminated the python dependancy. Shall I\nresend the patchset now or should I wait until it has been further\nreviewed? (don't want to flood the list with resubmits)\n\n - Nick\n\nOn Thu, Aug 6, 2009 at 9:06 PM, Johannes\nSchindelin wrote:\n> Hi,\n>\n> On Thu, 6 Aug 2009, Nick Edelen wrote:\n>\n>> > Sorry, I forgot the details, could you quickly remind me why these\n>> > caches are not in the pack index files?\n>>\n>> Er, I'm not sure what you mean.  Are you asking why these revision\n>> caches are required if we have a pack index, or why they aren't in the\n>> pack index, or something different?  I'm thinking probably the second:\n>\n> Yep.\n>\n>> the short answer is that cache slices are totally independant of pack\n>> files.\n>\n> My idea with that was that you already have a SHA-1 map in the pack index,\n> and if all you want to be able to accelerate the revision walker, you'd\n> probably need something that adds yet another mapping, from commit to\n> parents and tree, and from tree to sub-tree and blob (so you can avoid\n> unpacking commit and tree objects).\n>\n> I just thought that it could be more efficient to do it at the time the\n> pack index is written _anyway_, as nothing will change in the pack after\n> that anyway.\n>\n> But I guess I can answer my question easily myself: the boundary commits\n> will not be handled that way.\n>\n> Still, there is some redundancy between the pack index and your cache, as\n> you have to write out the whole list of SHA-1s all over again.  I guess it\n> is time to look at the code instead of asking stupid questions.\n>\n>> It might be possible to somehow merge revision cache slices with pack\n>> indexes, but I don't think it'd be a very suitable modification.  The\n>> rev-cache slices are meant to act almost like topo-relation pack files:\n>> new slices are simply new files, seperate slice files can be fused\n>> (\"repacked\") into a larger one, the index is a (recreatable) single file\n>> associating file (positions) with objects.  The format was geared to\n>> reducing potential cache/data loss and preventing overly large cache\n>> slices.\n>>\n>> >> Hmpf.\n>> >>\n>> >> We got rid of the last Python script in Git a long time ago, but now\n>> >> two different patch series try to sneak that dependency (at least for\n>> >> testing) back in.\n>> >>\n>> >> That's all the worse because we cannot use Python in msysGit, and\n>> >> Windows should be a platform benefitting dramatically from your work.\n>> >\n>> > In fact, the test the script performs could be easily rephrased with\n>> > \"sort\", \"uniq\" and \"comm\". OTOH: If the walker is supposed to return\n>> > the exact same orderd list of commits you can just use test_cmp.\n>>\n>> The language that script is written in isn't important.  I originally\n>> wrote it in python because I wanted something quick and wasn't much of\n>> a sh guru (sorry :-/ ).  As Micheal said I've no doubt it can easily\n>> be converted to shell script\n>\n> That is not what I wanted to hear.\n>\n>> -- in fact, I'll try to get a shell version working today.\n>\n> That is.\n>\n> Thanks,\n> Dscho\n>\n"},{"id":"119830","messageId":"c77435a80908061330h2461012at8b877970cab4906b@mail.gmail.com","threadId":"20427","inReplyTo":"c77435a80908061301n5e855aeci16af392ed3128651@mail.gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-06T20:30:41Z","receivedAt":"2009-08-06T20:30:41Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"Hrmm, I just realized that it dosn't actually cache paths/names...\nThis obviously has no bearing on its use in packing, but I should\neither add that in or restrict usage in non-packing-related walks.\nWeird how things like that escape you.\n\nI think I may go ahead and add support for this tomorrow. It should\nhave no effect on performance and very little impact on cache slice\nsize.\n\nOn Thu, Aug 6, 2009 at 10:01 PM, Nick Edelen wrote:\n> Hi,\n>\n>>My idea with that was that you already have a SHA-1 map in the pack index,\n>>and if all you want to be able to accelerate the revision walker, you'd\n>>probably need something that adds yet another mapping, from commit to\n>>parents and tree, and from tree to sub-tree and blob (so you can avoid\n>>unpacking commit and tree objects).\n>\n> As I mention in one of the patch descriptions, along with each commit\n> a list of objects introduced per commit is cached, so no extra I/O is\n> necessary for tree recursion, etc. during traversal.\n>\n>>I just thought that it could be more efficient to do it at the time the\n>>pack index is written _anyway_, as nothing will change in the pack after\n>>that anyway.\n>\n> Nothing might change in the pack, but the slices were made to allow\n> for continual addition and refinement of the cache.  In a typical\n> usage slices will be added and fused on a regular basis, which would\n> require tinkering in pack indexes if they were combined.\n>\n>>But I guess I can answer my question easily myself: the boundary commits\n>>will not be handled that way.\n>>\n>>Still, there is some redundancy between the pack index and your cache, as\n>>you have to write out the whole list of SHA-1s all over again.  I guess it\n>>is time to look at the code instead of asking stupid questions.\n>\n> The whole revision cache is redundant, technically speaking: nothing\n> in it can't be found by rummaging through packs or objects.  The point\n> of it was to distill out important information for fast, easy access\n> of the commit tree.\n>\n> On another note, I've eliminated the python dependancy.  Shall I\n> resend the patchset now or should I wait until it has been further\n> reviewed? (don't want to flood the list with resubmits)\n>\n>  - Nick\n>\n> On Thu, Aug 6, 2009 at 9:06 PM, Johannes\n> Schindelin wrote:\n>> Hi,\n>>\n>> On Thu, 6 Aug 2009, Nick Edelen wrote:\n>>\n>>> > Sorry, I forgot the details, could you quickly remind me why these\n>>> > caches are not in the pack index files?\n>>>\n>>> Er, I'm not sure what you mean.  Are you asking why these revision\n>>> caches are required if we have a pack index, or why they aren't in the\n>>> pack index, or something different?  I'm thinking probably the second:\n>>\n>> Yep.\n>>\n>>> the short answer is that cache slices are totally independant of pack\n>>> files.\n>>\n>> My idea with that was that you already have a SHA-1 map in the pack index,\n>> and if all you want to be able to accelerate the revision walker, you'd\n>> probably need something that adds yet another mapping, from commit to\n>> parents and tree, and from tree to sub-tree and blob (so you can avoid\n>> unpacking commit and tree objects).\n>>\n>> I just thought that it could be more efficient to do it at the time the\n>> pack index is written _anyway_, as nothing will change in the pack after\n>> that anyway.\n>>\n>> But I guess I can answer my question easily myself: the boundary commits\n>> will not be handled that way.\n>>\n>> Still, there is some redundancy between the pack index and your cache, as\n>> you have to write out the whole list of SHA-1s all over again.  I guess it\n>> is time to look at the code instead of asking stupid questions.\n>>\n>>> It might be possible to somehow merge revision cache slices with pack\n>>> indexes, but I don't think it'd be a very suitable modification.  The\n>>> rev-cache slices are meant to act almost like topo-relation pack files:\n>>> new slices are simply new files, seperate slice files can be fused\n>>> (\"repacked\") into a larger one, the index is a (recreatable) single file\n>>> associating file (positions) with objects.  The format was geared to\n>>> reducing potential cache/data loss and preventing overly large cache\n>>> slices.\n>>>\n>>> >> Hmpf.\n>>> >>\n>>> >> We got rid of the last Python script in Git a long time ago, but now\n>>> >> two different patch series try to sneak that dependency (at least for\n>>> >> testing) back in.\n>>> >>\n>>> >> That's all the worse because we cannot use Python in msysGit, and\n>>> >> Windows should be a platform benefitting dramatically from your work.\n>>> >\n>>> > In fact, the test the script performs could be easily rephrased with\n>>> > \"sort\", \"uniq\" and \"comm\". OTOH: If the walker is supposed to return\n>>> > the exact same orderd list of commits you can just use test_cmp.\n>>>\n>>> The language that script is written in isn't important.  I originally\n>>> wrote it in python because I wanted something quick and wasn't much of\n>>> a sh guru (sorry :-/ ).  As Micheal said I've no doubt it can easily\n>>> be converted to shell script\n>>\n>> That is not what I wanted to hear.\n>>\n>>> -- in fact, I'll try to get a shell version working today.\n>>\n>> That is.\n>>\n>> Thanks,\n>> Dscho\n>>\n>\n"},{"id":"119831","messageId":"20090806203223.GK1033@spearce.org","threadId":"20427","inReplyTo":"c77435a80908061330h2461012at8b877970cab4906b@mail.gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-06T20:32:23Z","receivedAt":"2009-08-06T20:32:23Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nick Edelen wrote:\n> Hrmm, I just realized that it dosn't actually cache paths/names...\n> This obviously has no bearing on its use in packing, but I should\n> either add that in or restrict usage in non-packing-related walks.\n> Weird how things like that escape you.\n> \n> I think I may go ahead and add support for this tomorrow. It should\n> have no effect on performance and very little impact on cache slice\n> size.\n\nYou may not need the path name, but instead the hash value that\npack-objects computes from the path name. All that matters is\nthe hash, so pack-objects can schedule the objects into the right\nbuckets when its doing delta computation for objects which are not\nyet delta compressed, or whose delta cannot be suitably reused.\n\n-- \nShawn.\n"},{"id":"119849","messageId":"4A7B68C4.8070406@gmail.com","threadId":"20427","inReplyTo":"20090806203223.GK1033@spearce.org","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2009-08-06T23:35:32Z","receivedAt":"2009-08-06T23:35:32Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Shawn O. Pearce wrote:\n> Nick Edelen wrote:\n>> Hrmm, I just realized that it dosn't actually cache paths/names...\n>> This obviously has no bearing on its use in packing, but I should\n>> either add that in or restrict usage in non-packing-related walks.\n>> Weird how things like that escape you.\n>>\n>> I think I may go ahead and add support for this tomorrow. It should\n>> have no effect on performance and very little impact on cache slice\n>> size.\n> \n> You may not need the path name, but instead the hash value that\n> pack-objects computes from the path name. All that matters is\n> the hash, so pack-objects can schedule the objects into the right\n> buckets when its doing delta computation for objects which are not\n> yet delta compressed, or whose delta cannot be suitably reused.\n> \n\nPlease do NOT expose the hash values. The hash used by pack-objects is \nan implementation detail of the heuristics used by the _current_ object \npacking code. It would be a real shame to have to maintain backward \ncompatibility with it at some future date after the packing machinery \nhas changed.\n"},{"id":"119850","messageId":"20090806233739.GL1033@spearce.org","threadId":"20427","inReplyTo":"4A7B68C4.8070406@gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-06T23:37:39Z","receivedAt":"2009-08-06T23:37:39Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"A Large Angry SCM wrote:\n> Shawn O. Pearce wrote:\n>> Nick Edelen wrote:\n>>> Hrmm, I just realized that it dosn't actually cache paths/names...\n>>\n>> You may not need the path name, but instead the hash value that\n>> pack-objects computes from the path name.\n>\n> Please do NOT expose the hash values. The hash used by pack-objects is \n> an implementation detail of the heuristics used by the _current_ object \n> packing code. It would be a real shame to have to maintain backward \n> compatibility with it at some future date after the packing machinery \n> has changed.\n\nThis is a local cache. If there was a version number in the header,\nand the hash function changes, we could just bump the version number\nand invalidate all of the caches.\n\nNo sense in storing (and doing IO of) huge duplicate string values\nfor something where we really only need 32 bits, and where a\nrecompute from scratch only costs a minute.\n\n-- \nShawn.\n"},{"id":"119852","messageId":"4A7B6A94.9020200@gmail.com","threadId":"20427","inReplyTo":"20090806233739.GL1033@spearce.org","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2009-08-06T23:43:16Z","receivedAt":"2009-08-06T23:43:16Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Shawn O. Pearce wrote:\n> A Large Angry SCM wrote:\n>> Shawn O. Pearce wrote:\n>>> Nick Edelen wrote:\n>>>> Hrmm, I just realized that it dosn't actually cache paths/names...\n>>> You may not need the path name, but instead the hash value that\n>>> pack-objects computes from the path name.\n>> Please do NOT expose the hash values. The hash used by pack-objects is \n>> an implementation detail of the heuristics used by the _current_ object \n>> packing code. It would be a real shame to have to maintain backward \n>> compatibility with it at some future date after the packing machinery \n>> has changed.\n> \n> This is a local cache. If there was a version number in the header,\n> and the hash function changes, we could just bump the version number\n> and invalidate all of the caches.\n> \n> No sense in storing (and doing IO of) huge duplicate string values\n> for something where we really only need 32 bits, and where a\n> recompute from scratch only costs a minute.\n> \n\nThat will work for me if the cache gets a version number and iff the \npack-objects hash code gets big warning comments about the cache code \ndependency.\n"},{"id":"119854","messageId":"c77435a80908061715u7ff7456cl6c5ded16b51b122f@mail.gmail.com","threadId":"20427","inReplyTo":"4A7B6A94.9020200@gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-07T00:15:17Z","receivedAt":"2009-08-07T00:15:17Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"That would work, but I sorta like the idea of caching the actual\nnames. I'm thinking of having a block of slice-unique, null-seperated\nnames at the end of each slice (ie. not in the mapping) which is\nloaded into memory (it wouldn't be very big). Then each blob/tree\nobject would have an variable length index referencing a particular\nname.\n\nUsing the actual names would give us greater flexbility, and allow\nrev-cache to output proper rev-list type output (with the names after\nthe hashes).\n\nOn Fri, Aug 7, 2009 at 1:43 AM, A Large Angry SCM wrote:\n> Shawn O. Pearce wrote:\n>>\n>> A Large Angry SCM wrote:\n>>>\n>>> Shawn O. Pearce wrote:\n>>>>\n>>>> Nick Edelen wrote:\n>>>>>\n>>>>> Hrmm, I just realized that it dosn't actually cache paths/names...\n>>>>\n>>>> You may not need the path name, but instead the hash value that\n>>>> pack-objects computes from the path name.\n>>>\n>>> Please do NOT expose the hash values. The hash used by pack-objects is\n>>>  an implementation detail of the heuristics used by the _current_ object\n>>>  packing code. It would be a real shame to have to maintain backward\n>>>  compatibility with it at some future date after the packing machinery  has\n>>> changed.\n>>\n>> This is a local cache.  If there was a version number in the header,\n>> and the hash function changes, we could just bump the version number\n>> and invalidate all of the caches.\n>>\n>> No sense in storing (and doing IO of) huge duplicate string values\n>> for something where we really only need 32 bits, and where a\n>> recompute from scratch only costs a minute.\n>>\n>\n> That will work for me if the cache gets a version number and iff the\n> pack-objects hash code gets big warning comments about the cache code\n> dependency.\n>\n"},{"id":"119873","messageId":"4A7B95A8.2010000@vilain.net","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908062030340.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-08-07T02:47:04Z","receivedAt":"2009-08-07T02:47:04Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n>> the short answer is that cache slices are totally independant of pack \n>> files.\n>> \n>\n> My idea with that was that you already have a SHA-1 map in the pack index, \n> and if all you want to be able to accelerate the revision walker, you'd \n> probably need something that adds yet another mapping, from commit to \n> parents and tree, and from tree to sub-tree and blob (so you can avoid \n> unpacking commit and tree objects).\n> \n\nTying indexes together like that is not a good idea in the database\nworld. Especially as in this case as Nick mentions, the domain is subtly\ndifferent (ie pack vs dag). Unfortunately you just can't try to pretend\nthat they will always be the same; you can't force a full repack on\nevery ref change!\n\n> Still, there is some redundancy between the pack index and your cache, as \n> you have to write out the whole list of SHA-1s all over again. I guess it \n> is time to look at the code instead of asking stupid questions.\n> \n\n\"Disk is cheap\" :-) It should be a welcome trade-off; perhaps it's worth\nincluding numbers about how big the indexes are with the time\nstatistics. It sounds though like it should be a significant win as a\nsingle index can be used to accelerate a wide range of rev-list\noperations, and store indexes for many different questions that can be\nasked.\n\nSam\n"},{"id":"119882","messageId":"alpine.LFD.2.00.0908070031160.16073@xanadu.home","threadId":"20427","inReplyTo":"4A7B95A8.2010000@vilain.net","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-07T04:35:35Z","receivedAt":"2009-08-07T04:35:35Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 7 Aug 2009, Sam Vilain wrote:\n\n> Johannes Schindelin wrote:\n> >> the short answer is that cache slices are totally independant of pack \n> >> files.\n> >> \n> >\n> > My idea with that was that you already have a SHA-1 map in the pack index, \n> > and if all you want to be able to accelerate the revision walker, you'd \n> > probably need something that adds yet another mapping, from commit to \n> > parents and tree, and from tree to sub-tree and blob (so you can avoid \n> > unpacking commit and tree objects).\n> > \n> \n> Tying indexes together like that is not a good idea in the database\n> world. Especially as in this case as Nick mentions, the domain is subtly\n> different (ie pack vs dag). Unfortunately you just can't try to pretend\n> that they will always be the same; you can't force a full repack on\n> every ref change!\n\nRight. And the rev cache must work even if the repository is not \npacked. So pack index and rev caching are orthogonal things and are best \nkept separate on disk.\n\nHow big this cache might get would be interesting indeed.\n\n\nNicolas\n"},{"id":"119884","messageId":"alpine.LFD.2.00.0908070036450.16073@xanadu.home","threadId":"20427","inReplyTo":"c77435a80908061330h2461012at8b877970cab4906b@mail.gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-07T04:42:14Z","receivedAt":"2009-08-07T04:42:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 6 Aug 2009, Nick Edelen wrote:\n\n> Hrmm, I just realized that it dosn't actually cache paths/names...\n> This obviously has no bearing on its use in packing, but I should\n> either add that in or restrict usage in non-packing-related walks.\n> Weird how things like that escape you.\n\nActually it is really the packing related walk that would benefit the \nmost from this work. The \"counting objects\" phase of a clone may take \nquite a while with some repositories. Most other operations don't care \nas much because the rev walk is done incrementally whereas the packing \noperation needs to perform it all up front.\n\n\nNicolas\n"},{"id":"119891","messageId":"alpine.DEB.1.00.0908070801540.8306@pacific.mpi-cbg.de","threadId":"20427","inReplyTo":"20090806233739.GL1033@spearce.org","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-07T06:05:25Z","receivedAt":"2009-08-07T06:05:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n\nOn Thu, 6 Aug 2009, Shawn O. Pearce wrote:\n\n> A Large Angry SCM wrote:\n> > Shawn O. Pearce wrote:\n> >> Nick Edelen wrote:\n> >>> Hrmm, I just realized that it dosn't actually cache paths/names...\n> >>\n> >> You may not need the path name, but instead the hash value that \n> >> pack-objects computes from the path name.\n> >\n> > Please do NOT expose the hash values. The hash used by pack-objects is \n> > an implementation detail of the heuristics used by the _current_ \n> > object packing code. It would be a real shame to have to maintain \n> > backward compatibility with it at some future date after the packing \n> > machinery has changed.\n> \n> This is a local cache. If there was a version number in the header, and \n> the hash function changes, we could just bump the version number and \n> invalidate all of the caches.\n> \n> No sense in storing (and doing IO of) huge duplicate string values for \n> something where we really only need 32 bits, and where a recompute from \n> scratch only costs a minute.\n\nFWIW it was this redundancy in duplicate (unpacked) string redundancy \nI meant, but I did a poor job at expressing myself, and consequently Nick \ndid not understand what I want (and I'm on a slow connection, so I deleted \nthe mail halfway through looking if there are some real answers hidden in \nthe huge quoted part instead of replying).\n\nAnd the fragility of the dependency to the implementation detail of the \npack index suggests to me that my intuition that this whole thing should \nbe more tightly integrated with the pack index was not totally off the \nmark.\n\nCiao,\nDscho\n"},{"id":"119892","messageId":"alpine.DEB.1.00.0908070806350.8306@pacific.mpi-cbg.de","threadId":"20427","inReplyTo":"alpine.LFD.2.00.0908070031160.16073@xanadu.home","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-07T06:08:21Z","receivedAt":"2009-08-07T06:08:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 7 Aug 2009, Nicolas Pitre wrote:\n\n> On Fri, 7 Aug 2009, Sam Vilain wrote:\n> \n> > Johannes Schindelin wrote:\n> > >> the short answer is that cache slices are totally independant of \n> > >> pack files.\n> > >> \n> > >\n> > > My idea with that was that you already have a SHA-1 map in the pack \n> > > index, and if all you want to be able to accelerate the revision \n> > > walker, you'd probably need something that adds yet another mapping, \n> > > from commit to parents and tree, and from tree to sub-tree and blob \n> > > (so you can avoid unpacking commit and tree objects).\n> > > \n> > \n> > Tying indexes together like that is not a good idea in the database \n> > world. Especially as in this case as Nick mentions, the domain is \n> > subtly different (ie pack vs dag). Unfortunately you just can't try to \n> > pretend that they will always be the same; you can't force a full \n> > repack on every ref change!\n> \n> Right. And the rev cache must work even if the repository is not \n> packed.\n\nUmm, why? AFAICT the principal purpose of the rev cache is to help work \nloads on, say, www.kernel.org.\n\nI am unlikely to notice the improvements in my regular \"git log\" calls \nthat only show a couple of pages before I quit the pager.\n\nCiao,\nDscho\n"},{"id":"119893","messageId":"alpine.DEB.1.00.0908070808340.8306@pacific.mpi-cbg.de","threadId":"20427","inReplyTo":"4A7B95A8.2010000@vilain.net","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-07T06:12:34Z","receivedAt":"2009-08-07T06:12:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 7 Aug 2009, Sam Vilain wrote:\n\n> Johannes Schindelin wrote:\n> >> the short answer is that cache slices are totally independant of pack \n> >> files.\n> >> \n> >\n> > My idea with that was that you already have a SHA-1 map in the pack \n> > index, and if all you want to be able to accelerate the revision \n> > walker, you'd probably need something that adds yet another mapping, \n> > from commit to parents and tree, and from tree to sub-tree and blob \n> > (so you can avoid unpacking commit and tree objects).\n> > \n> \n> Tying indexes together like that is not a good idea in the database\n> world.\n\nThis is not the same index as in the database world. It is more \ncomparable with a cached view. And there, it is generally a good idea to \nkeep related things in the same cached view (with an outer join), rather \nthan having two primary keys for almost every record.\n\n> Especially as in this case as Nick mentions, the domain is subtly \n> different (ie pack vs dag). Unfortunately you just can't try to pretend \n> that they will always be the same; you can't force a full repack on \n> every ref change!\n\nNo, but you do not need that, either. In the setting that is most likely \nthe most thankful one, i.e. a git:// server, you _want_ to keep the \nrepository \"as packed as possible\", otherwise the rev cache improvements \nwill be lost in the bad packing performance anyway.\n\n> > Still, there is some redundancy between the pack index and your cache, \n> > as you have to write out the whole list of SHA-1s all over again. I \n> > guess it is time to look at the code instead of asking stupid \n> > questions.\n> > \n> \n> \"Disk is cheap\" :-)\n\nDisk I/O ain't.\n\n(Size of the I/O caches, yaddayadda, I'm sure you get my point).\n\nCiao,\nDscho\n"},{"id":"119906","messageId":"alpine.LFD.2.00.0908071010580.16073@xanadu.home","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908070806350.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-07T14:18:14Z","receivedAt":"2009-08-07T14:18:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 7 Aug 2009, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Fri, 7 Aug 2009, Nicolas Pitre wrote:\n> \n> > On Fri, 7 Aug 2009, Sam Vilain wrote:\n> > \n> > > Johannes Schindelin wrote:\n> > > >> the short answer is that cache slices are totally independant of \n> > > >> pack files.\n> > > >> \n> > > >\n> > > > My idea with that was that you already have a SHA-1 map in the pack \n> > > > index, and if all you want to be able to accelerate the revision \n> > > > walker, you'd probably need something that adds yet another mapping, \n> > > > from commit to parents and tree, and from tree to sub-tree and blob \n> > > > (so you can avoid unpacking commit and tree objects).\n> > > > \n> > > \n> > > Tying indexes together like that is not a good idea in the database \n> > > world. Especially as in this case as Nick mentions, the domain is \n> > > subtly different (ie pack vs dag). Unfortunately you just can't try to \n> > > pretend that they will always be the same; you can't force a full \n> > > repack on every ref change!\n> > \n> > Right. And the rev cache must work even if the repository is not \n> > packed.\n> \n> Umm, why? AFAICT the principal purpose of the rev cache is to help work \n> loads on, say, www.kernel.org.\n\nSo what?\n\nSpeeding up rev-list with a rev cache is completely orthogonal to \nwhether the repository is packed or not. It is like having a \"git diff\" \nresult cache: no one would think of stuffing that in the pack index.\n\nIf we want to improve on the repository packing format, that must be \ndoable without bothering with an independent concept such as a rev \ncache.\n\n> I am unlikely to notice the improvements in my regular \"git log\" calls \n> that only show a couple of pages before I quit the pager.\n\nIndeed. But what is your point again?\n\n\nNicolas\n"},{"id":"119908","messageId":"alpine.LFD.2.00.0908071029580.16073@xanadu.home","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908070808340.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-07T15:00:42Z","receivedAt":"2009-08-07T15:00:42Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 7 Aug 2009, Johannes Schindelin wrote:\n\n> On Fri, 7 Aug 2009, Sam Vilain wrote:\n> \n> > Especially as in this case as Nick mentions, the domain is subtly \n> > different (ie pack vs dag). Unfortunately you just can't try to pretend \n> > that they will always be the same; you can't force a full repack on \n> > every ref change!\n> \n> No, but you do not need that, either. In the setting that is most likely \n> the most thankful one, i.e. a git:// server, you _want_ to keep the \n> repository \"as packed as possible\", otherwise the rev cache improvements \n> will be lost in the bad packing performance anyway.\n\nYes and no.\n\nCurrently, the number #1 latency in any initial git clone is the famous \n\"counting objects\" phase, even if the repo is perfectly packed. And \nthat's all this rev cache can and will improve. The packing does play \nits performance role of course, but for a totally different reason. \nHence the repository needs no be perfectly packed for a rev cache to \nspeed up its own part of the game.\n\n> > > Still, there is some redundancy between the pack index and your cache, \n> > > as you have to write out the whole list of SHA-1s all over again. I \n> > > guess it is time to look at the code instead of asking stupid \n> > > questions.\n> > > \n> > \n> > \"Disk is cheap\" :-)\n> \n> Disk I/O ain't.\n> \n> (Size of the I/O caches, yaddayadda, I'm sure you get my point).\n\nI don't know about the size of the rev cache on disk yet (I asked Nick \nabout that) nor do I really know how this cache is implemented. But I \nknow damn well about git packs and associated index and I for sure don't \nwant to see a revision cache coupled with it.\n\nAnd for a clone the disk IO will certainly be a magnitude larger than \nfor the cache (or so I hope). Maybe the IO for the rev cache might be a \nsignificant overhead for operations performed on a client (aka \ndeveloper) repository, in which case it would be a good idea to have a \nconfig variable to control the cache size, or even to turn it off \nentirely. We do it for delta depth and many other things already.\n\n\nNicolas\n"},{"id":"119940","messageId":"c77435a80908071502y48d14a38h79eec14a1be8c6fb@mail.gmail.com","threadId":"20427","inReplyTo":"alpine.LFD.2.00.0908071029580.16073@xanadu.home","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-07T22:02:57Z","receivedAt":"2009-08-07T22:02:57Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"> I don't know about the size of the rev cache on disk yet (I asked Nick\n> about that) nor do I really know how this cache is implemented.\n\nThe cache file for all of the linux repository (as of a few weeks ago)\nis around 42MB, without names. The names would probably add 2 or 3 MB\non top of that. That's probably about as big as I'd want to get, as\nthe whole slice (minus the name list) is mapped to memory (then again\nbigger might be ok; I'm not an expert on mem mapping). The rev-cache\ncommand fuse provides functionality to ignore certain cache sizes,\nwhich was geared towards preventing overly large slices.\n"},{"id":"119941","messageId":"7vzlabp7e4.fsf@alter.siamese.dyndns.org","threadId":"20427","inReplyTo":"c77435a80908071502y48d14a38h79eec14a1be8c6fb@mail.gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-07T22:48:51Z","receivedAt":"2009-08-07T22:48:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Edelen writes:\n\n> The cache file for all of the linux repository (as of a few weeks ago)\n> is around 42MB, without names. The names would probably add 2 or 3 MB\n> on top of that. That's probably about as big as I'd want to get,\n\nHmm. .git/objects/ as of today is about 482M here, so we are talking\nabout roughly 10% overhead?\n"},{"id":"119942","messageId":"c77435a80908071553m6f7d5298p5ea68b9386198b3f@mail.gmail.com","threadId":"20427","inReplyTo":"7vzlabp7e4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-07T22:53:12Z","receivedAt":"2009-08-07T22:53:12Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"> Hmm.  .git/objects/ as of today is about 482M here, so we are talking\n> about roughly 10% overhead?\n\nYes, that sounds about right. The cache file for git's repository is\n3MB, and my repo (partly packed) is ~35MB.\n\nBy the way, what would be the best way of posting a revised patchset?\nShould I just reply to my older posts, or make new ones?\n"},{"id":"119945","messageId":"20090808025055.GA14731@coredump.intra.peff.net","threadId":"20427","inReplyTo":"7vzlabp7e4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-08T02:50:55Z","receivedAt":"2009-08-08T02:50:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 07, 2009 at 03:48:51PM -0700, Junio C Hamano wrote:\n\n> > The cache file for all of the linux repository (as of a few weeks ago)\n> > is around 42MB, without names. The names would probably add 2 or 3 MB\n> > on top of that. That's probably about as big as I'd want to get,\n> \n> Hmm. .git/objects/ as of today is about 482M here, so we are talking\n> about roughly 10% overhead?\n\nIIRC from previous discussions, kernel.org's main performance problem is\nI/O, not CPU. Are there any provisions for sharing rev-caches between\nsimilar repositories, as we already do for objects?\n\n-Peff\n"},{"id":"119946","messageId":"7vfxc3ov8t.fsf@alter.siamese.dyndns.org","threadId":"20427","inReplyTo":"c77435a80908071553m6f7d5298p5ea68b9386198b3f@mail.gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-08T03:11:14Z","receivedAt":"2009-08-08T03:11:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Edelen writes:\n\n> By the way, what would be the best way of posting a revised patchset?\n> Should I just reply to my older posts, or make new ones?\n\nThat depends primarily on how heavily the patches needed to change in\nresponse to review comments, but until the series lands in 'next', you\nwould typically send updated series as a replacement, not incremental.\n\nMany people seemed to be interested in the series and had a volume of\ncomments on it. I suspect the updated series would be quite different\nfrom the original, so for the next round I would suspect it would be best\nto start anew, marking them as [PATCH N/M (v2)], in a fresh thread. It\nwould help reviewers if you said \"this corresponds to [PATCH 3/5] in the\noriginal series, with the following improvements based on X and Y's\ncomments\" after the three-dash line.\n"},{"id":"119960","messageId":"c77435a80908080027p6a8de468w68983bd81364cefd@mail.gmail.com","threadId":"20427","inReplyTo":"7vfxc3ov8t.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-08T07:27:55Z","receivedAt":"2009-08-08T07:27:55Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"> IIRC from previous discussions, kernel.org's main performance problem is\n> I/O, not CPU. Are there any provisions for sharing rev-caches between\n> similar repositories, as we already do for objects?\n\nI haven't implemented a transmission protocol or anything, but it\nwould be perfectly possible to copy cache slices from one repo to\nanother. Generating the revision cache from scratch on large repos\ncan take several minutes, so this wouldn't be a bad idea.\n\n> That depends primarily on how heavily the patches needed to change in\n> response to review comments, but until the series lands in 'next', you\n> would typically send updated series as a replacement, not incremental.\n>\n> Many people seemed to be interested in the series and had a volume of\n> comments on it.  I suspect the updated series would be quite different\n> from the original, so for the next round I would suspect it would be best\n> to start anew, marking them as [PATCH N/M (v2)], in a fresh thread.  It\n> would help reviewers if you said \"this corresponds to [PATCH 3/5] in the\n> original series, with the following improvements based on X and Y's\n> comments\" after the three-dash line.\n\nOk, that sounds good. I've added a new patch as well so the numbering changes.\n"},{"id":"119961","messageId":"20090808073042.GA20951@coredump.intra.peff.net","threadId":"20427","inReplyTo":"c77435a80908080027p6a8de468w68983bd81364cefd@mail.gmail.com","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-08T07:30:42Z","receivedAt":"2009-08-08T07:30:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 08, 2009 at 09:27:55AM +0200, Nick Edelen wrote:\n\n> > IIRC from previous discussions, kernel.org's main performance problem is\n> > I/O, not CPU. Are there any provisions for sharing rev-caches between\n> > similar repositories, as we already do for objects?\n> \n> I haven't implemented a transmission protocol or anything, but it\n> would be perfectly possible to copy cache slices from one repo to\n> another. Generating the revision cache from scratch on large repos\n> can take several minutes, so this wouldn't be a bad idea.\n\nThat might be useful, but I was thinking more of an \"alternates\"-like\nmechanism between repos. So that the data is stored only once on disk\nand in the disk cache, which is helpful for sites like kernel.org which\nserve many similar repositories.\n\n-Peff\n"},{"id":"119969","messageId":"c77435a80908080040l4c92c68du777fbff5a9a2aca2@mail.gmail.com","threadId":"20427","inReplyTo":"20090808073042.GA20951@coredump.intra.peff.net","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-08T07:40:39Z","receivedAt":"2009-08-08T07:40:39Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"> That might be useful, but I was thinking more of an \"alternates\"-like\n> mechanism between repos. So that the data is stored only once on disk\n> and in the disk cache, which is helpful for sites like kernel.org which\n> serve many similar repositories.\n\nOh, right. Yes, that seems like it could work. We'd have to be\ncareful that a shared cache slice wouldn' change (like in a fuse or\nsomething), but other than that we could have something as simple as a\nlink.\n"},{"id":"119988","messageId":"alpine.DEB.1.00.0908081709380.8306@pacific.mpi-cbg.de","threadId":"20427","inReplyTo":"alpine.LFD.2.00.0908071010580.16073@xanadu.home","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-08T15:18:42Z","receivedAt":"2009-08-08T15:18:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 7 Aug 2009, Nicolas Pitre wrote:\n\n> On Fri, 7 Aug 2009, Johannes Schindelin wrote:\n> \n> > Hi,\n> > \n> > On Fri, 7 Aug 2009, Nicolas Pitre wrote:\n> > \n> > > On Fri, 7 Aug 2009, Sam Vilain wrote:\n> > > \n> > > > Johannes Schindelin wrote:\n> > > > >> the short answer is that cache slices are totally independant of \n> > > > >> pack files.\n> > > > >> \n> > > > >\n> > > > > My idea with that was that you already have a SHA-1 map in the pack \n> > > > > index, and if all you want to be able to accelerate the revision \n> > > > > walker, you'd probably need something that adds yet another mapping, \n> > > > > from commit to parents and tree, and from tree to sub-tree and blob \n> > > > > (so you can avoid unpacking commit and tree objects).\n> > > > > \n> > > > \n> > > > Tying indexes together like that is not a good idea in the database \n> > > > world. Especially as in this case as Nick mentions, the domain is \n> > > > subtly different (ie pack vs dag). Unfortunately you just can't try to \n> > > > pretend that they will always be the same; you can't force a full \n> > > > repack on every ref change!\n> > > \n> > > Right. And the rev cache must work even if the repository is not \n> > > packed.\n> > \n> > Umm, why? AFAICT the principal purpose of the rev cache is to help work \n> > loads on, say, www.kernel.org.\n> \n> So what?\n> \n> Speeding up rev-list with a rev cache is completely orthogonal to \n> whether the repository is packed or not.\n\nNo, it is not.\n\nFor both technical and practical reasons, caching revision walker data is\nvery closely related to packing.\n\nYou are _very_ unlikely helped by speeding up revision walking in the \ngeneral case, _especially_ when you do stuff like blame or -S that needs \nto unpack tons of objects _anyway_.\n\nThe one big kicker argument for speeding up revision walking _is_ to \nrelieve the loads on big ass servers, and they _should_ be as packed as \npossible (as I will patiently explain over and over again).\n\n> It is like having a \"git diff\" result cache: no one would think of \n> stuffing that in the pack index.\n\nDo you want to try to kid me? You'll have to try harder. Caching \"git \ndiff\" results... no, really!\n\n> If we want to improve on the repository packing format, that must be \n> doable without bothering with an independent concept such as a rev \n> cache.\n\nI would love to tell you that you're right, but the single fact that \npack v4 is startig to compete with Duke Nukem Forever just prevents me \nfrom doing that.\n\n> > I am unlikely to notice the improvements in my regular \"git log\" calls \n> > that only show a couple of pages before I quit the pager.\n> \n> Indeed. But what is your point again?\n\nOh? My point? Being that the rev cache has a certain target audience, \nand that the regular user is not part of that audience, and that it just \nso happens that the _technical_ similarities with the pack index can be \nexploited in those scenarios?\n\nIOW we can be pretty certain that a heavy-load server has a fully (or \nnext-to-fully) packed object database. The pack indices already contain a \nSHA-1 table that we can simply reuse. And it should not be hard (or \nfragile) at all to put the \"cached\" information about parents, \nreferenced tree and blob objects into that file, into a different section.\n\nAfter all, the parents, referenced tree and blob objects to change as \noften as the objects in the pack: never.\n\nCiao,\nDscho\n"},{"id":"119991","messageId":"7vk51el25g.fsf@alter.siamese.dyndns.org","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908081709380.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-08T16:07:55Z","receivedAt":"2009-08-08T16:07:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> It is like having a \"git diff\" result cache: no one would think of \n>> stuffing that in the pack index.\n>\n> Do you want to try to kid me? You'll have to try harder. Caching \"git \n> diff\" results... no, really!\n\nI thought Nico meant caching the rename similarity matrix when he said\n\"git diff\" cache. In a narrow context of \"log --follow\" it may make\nsense. It also might help \"blame\" without -C.\n\nBut I do not see how we would gain anything if we had that cache tied to\nthe pack index.\n\n> After all, the parents, referenced tree and blob objects to change as \n> often as the objects in the pack: never.\n\nBut I notice that the aspects of objects you listed: the parents,\nreferenced tree and blob objects. The frequency of them changing does not\ndepend on where the actual object is, either packed or loose. These\naspects of an object (more specifically, you are talking about a commit\nobject) never change either way. So I am somewhat puzzled.\n\nIt does change if a particular commit and its associated objects are\nrelevant to the traversal as refs change (especially when they rewind).\nJust like an old \"kept\" pack can suddenly have tons of irrelevant objects\nafter refs are pruned (e.g. a branch is dropped), cached reachability\ndata, even though they may stay correct, would become irrelevant when\nnobody starts traversing from an object that is no longer reachable.\n"},{"id":"120005","messageId":"7vmy6agmlj.fsf@alter.siamese.dyndns.org","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908062030340.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-08T18:57:28Z","receivedAt":"2009-08-08T18:57:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> My idea with that was that you already have a SHA-1 map in the pack index, \n> and if all you want to be able to accelerate the revision walker, you'd \n> probably need something that adds yet another mapping, from commit to \n> parents and tree, and from tree to sub-tree and blob (so you can avoid \n> unpacking commit and tree objects).\n>\n> I just thought that it could be more efficient to do it at the time the \n> pack index is written _anyway_, as nothing will change in the pack after \n> that anyway.\n\nAfter reading the version 2 of the \"documentation\" patch and commenting\nheavily on it, I partly share the same feeling with you. The codepath to\npack objects is _one of the places_ you can generate rev-cache and slice\ninformation without redoing a lot of work that has already been done\nanyway.\n\nBut\n\n - You can write that information separately out to a different file.\n Logically it does not have to be _in_ the same pack idx file; and\n\n - You may want to generate rev-cache information even if you do not pack\n the repository. They may practically go hand-in-hand, but logically\n they are orthogonal.\n\nAnd I am not sure if it is easy to retrofit \"rev-list | pack-objects\" code\nto additionally produce this information, while keeping the standalone\nversion of rev-cache generation.\n\nHaving said all that.\n\nI haven't read the side of the patch that _uses_ the information stored in\nthe rev-cache to figure out what it optimizes and what its limitations are\n(e.g. how it interacts with pathspecs). Perhaps the rev-cache may turn\nout to be _only_ useful for pack-objects and nothing else, in which case\nwe may not care about standalone version of rev-cache generator after all.\n\nIf that is the case, I think it is also a reasonable implementation if the\nrev-cache is generated only by \"rev-list | pack-objects\" codepath as a\nside effect of traversal it already does, and it might even make sense to\nintroduce the version 3 of pack idx format that let you record additional\ninformation, like you suggest. I am not ready to make that judgement as I\nhaven't read the rest, but my gut feeling tells me that you might be\nright.\n"},{"id":"120021","messageId":"1249775658.7114.66.camel@maia.lan","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908081709380.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-08-08T23:54:18Z","receivedAt":"2009-08-08T23:54:18Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On Sat, 2009-08-08 at 17:18 +0200, Johannes Schindelin wrote:\n> > Speeding up rev-list with a rev cache is completely orthogonal to \n> > whether the repository is packed or not.\n> \n> No, it is not.\n> \n> For both technical and practical reasons, caching revision walker data is\n> very closely related to packing.\n> [...]\n> ... the rev cache has a certain target audience, \n> and that the regular user is not part of that audience, and that it just \n> so happens that the _technical_ similarities with the pack index can be \n> exploited in those scenarios?\n> \n> IOW we can be pretty certain that a heavy-load server has a fully (or \n> next-to-fully) packed object database. The pack indices already contain a \n> SHA-1 table that we can simply reuse. And it should not be hard (or \n> fragile) at all to put the \"cached\" information about parents, \n> referenced tree and blob objects into that file, into a different section.\n\nI think your argument would work better if packs and bundles were the\nsame thing, and we always stored bundles in the objects/packs directory,\nbut they're not and we don't. You can't assume that a pack has any\nparticular properties, such as representing the objects returned from a\nsingle rev-cache walk. And I will say that *especially* on a busy git\nserver, serving active projects you can't expect people to repack their\nrepository for every single update. Repacking daily or so by a batch\njob, sure. Expecting the repository to always be fully packed? No.\nToo much churn, or inefficient packing. You can't just pretend that the\nmixed packed/loose case doesn't exist.\n\nThe 10% size seems a very good bang for your buck to me and a good\nstart.\n\nSam\n"},{"id":"120025","messageId":"alpine.LFD.2.00.0908082147480.32635@xanadu.home","threadId":"20427","inReplyTo":"alpine.DEB.1.00.0908081709380.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-09T02:37:05Z","receivedAt":"2009-08-09T02:37:05Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 8 Aug 2009, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Fri, 7 Aug 2009, Nicolas Pitre wrote:\n> \n> > On Fri, 7 Aug 2009, Johannes Schindelin wrote:\n> > \n> > > Hi,\n> > > \n> > > On Fri, 7 Aug 2009, Nicolas Pitre wrote:\n> > > \n> > > > On Fri, 7 Aug 2009, Sam Vilain wrote:\n> > > > \n> > > > > Johannes Schindelin wrote:\n> > > > > >> the short answer is that cache slices are totally independant of \n> > > > > >> pack files.\n> > > > > >> \n> > > > > >\n> > > > > > My idea with that was that you already have a SHA-1 map in the pack \n> > > > > > index, and if all you want to be able to accelerate the revision \n> > > > > > walker, you'd probably need something that adds yet another mapping, \n> > > > > > from commit to parents and tree, and from tree to sub-tree and blob \n> > > > > > (so you can avoid unpacking commit and tree objects).\n> > > > > > \n> > > > > \n> > > > > Tying indexes together like that is not a good idea in the database \n> > > > > world. Especially as in this case as Nick mentions, the domain is \n> > > > > subtly different (ie pack vs dag). Unfortunately you just can't try to \n> > > > > pretend that they will always be the same; you can't force a full \n> > > > > repack on every ref change!\n> > > > \n> > > > Right. And the rev cache must work even if the repository is not \n> > > > packed.\n> > > \n> > > Umm, why? AFAICT the principal purpose of the rev cache is to help work \n> > > loads on, say, www.kernel.org.\n> > \n> > So what?\n> > \n> > Speeding up rev-list with a rev cache is completely orthogonal to \n> > whether the repository is packed or not.\n> \n> No, it is not.\n> \n> For both technical and practical reasons, caching revision walker data is\n> very closely related to packing.\n\nNo it is not.\n\n> You are _very_ unlikely helped by speeding up revision walking in the \n> general case, _especially_ when you do stuff like blame or -S that needs \n> to unpack tons of objects _anyway_.\n\nI completely agree. And this is why I wish for the rev cache to be \nenabled with a config variable. Why? Because \"client\" repositories are \nunlikely to benefit as much as \"server\" repositories from this cache.\n\n> The one big kicker argument for speeding up revision walking _is_ to \n> relieve the loads on big ass servers, and they _should_ be as packed as \n> possible (as I will patiently explain over and over again).\n\nPlease do a shortlog on builtin-pack-objects.c and realize who spent a \nlot of energy making git repository packing what it is now.\n\nThen re-read what I said on this list for more than a year now about the \nremaining latency on large git clone operations. After explaining it a \ncouple times already I will need some of your patience to make people \nunderstand that fully packing your repo is _not_ going to help it as \nwe're always talking about fully packed repos to start with.\n\n> > It is like having a \"git diff\" result cache: no one would think of \n> > stuffing that in the pack index.\n> \n> Do you want to try to kid me? You'll have to try harder. Caching \"git \n> diff\" results... no, really!\n\nWhy not? You worked on git diff yourself, so you certainly are well \npositionned to appreciate my suggestion, no?\n\n> > If we want to improve on the repository packing format, that must be \n> > doable without bothering with an independent concept such as a rev \n> > cache.\n> \n> I would love to tell you that you're right, but the single fact that \n> pack v4 is startig to compete with Duke Nukem Forever just prevents me \n> from doing that.\n\nBecause of the current economy, I was waiting to be laid off so to have \nthe time to make pack v4 a reality. Unfortunately for git they didn't \nfire me yet. Oh well...\n\n> > > I am unlikely to notice the improvements in my regular \"git log\" calls \n> > > that only show a couple of pages before I quit the pager.\n> > \n> > Indeed. But what is your point again?\n> \n> Oh? My point? Being that the rev cache has a certain target audience, \n> and that the regular user is not part of that audience, and that it just \n> so happens that the _technical_ similarities with the pack index can be \n> exploited in those scenarios?\n\nI don't see a similarity with the pack index at all, certainly not a \ntechnical one.\n\n> IOW we can be pretty certain that a heavy-load server has a fully (or \n> next-to-fully) packed object database. The pack indices already contain a \n> SHA-1 table that we can simply reuse. And it should not be hard (or \n> fragile) at all to put the \"cached\" information about parents, \n> referenced tree and blob objects into that file, into a different section.\n\nAnd then someone does a few pushes. So most of the time your repository \ncontains a few packs and not only a single one. So in which pack index \nfiles should you put the rev cache? What do yo do with that cache if it \nhappens to be split across multiple pack index files when a repack is \nperformed? Can't you see all the disadvantages to tie rev cache data \nwhich happens to share no issue with repacking into the same file?\n\nSo what do you do to keep the code simple and maintainable? Yes, you \nabstract things and use separate files, thank you very much. After all, \nthe packed refs are not stored in the pack index file either even if the \npacked-refs file contains a list of SHA1's.\n\n\nNicolas\n"},{"id":"120052","messageId":"c77435a80908090642g6d473c02o2e36a5a035e03b87@mail.gmail.com","threadId":"20427","inReplyTo":"alpine.LFD.2.00.0908082147480.32635@xanadu.home","subject":"Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-08-09T13:42:12Z","receivedAt":"2009-08-09T13:42:12Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"I can see the logic behind Johannes's ideas, but I'm still not sure\nit'd be a great modification. If you wanted to associate revision\ncaching much more strongly with packs, then packs and slices could be\nmerged reasonably well. Say you just attached the actual slice data\nat the end of the pack, then stored offsets of the slice payload in\nthe pack index. Since you'd (presumably) have to search the index for\nthe object anyway, you wouldn't have to deal with searching a\nrev-cache index on top of that (although it's not exactly unoptimized\nnow).\n\nHowever, that would sorta be preemptively limiting rev-cache to\npack-related optimizations. I mean at the moment that's the main\ntarget, but it could be improved in the future to be more relavant to\nother operations as well. Leaving the rev-cache as a seperate system\nwould keep both it and packing much more flexible, and open to\nlonger-term developments.\n\n>I haven't read the side of the patch that _uses_ the information stored in\n>the rev-cache to figure out what it optimizes and what its limitations are\n>(e.g. how it interacts with pathspecs). Perhaps the rev-cache may turn\n>out to be _only_ useful for pack-objects and nothing else, in which case\n>we may not care about standalone version of rev-cache generator after all.\n\nrev-cache's cache slice traversal basically emulates git's revision\nwalker, on a smaller scale. At the moment it only really handles date\nlimiting (and obviously slop stuff) so it's not used for any pruning.\nThat's not to say it couldn't be updated in the future though.\n"}]}