{"thread":{"id":"43298","subject":"[PATCH take 2] change the unpack limit treshold to a saner value","startedAt":"2006-12-06T21:08:56Z","lastAt":"2006-12-07T07:59:56Z","messageCount":8,"participants":["Nicolas Pitre","Junio C Hamano","Linus Torvalds","Shawn Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"298640","messageId":"Pine.LNX.4.64.0612061420410.2630@xanadu.home","threadId":"43298","inReplyTo":null,"subject":"[PATCH] change the unpack limit treshold to a saner value","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-12-06T21:08:56Z","receivedAt":"2006-12-06T21:08:56Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"\nLet's assume the average object size is x. Given n objects, the needed \nstorage size is n*(x + b), where b is the average wasted block size on \ndisk.\n\nIf those objects are packed, the needed storage becomes n*x + b so we \nsave on the block overhead.  But there is the pack index which is \n1024 + n*24 + b.\n\nTrying to find the value of n where packed objects become an advantage, \nwe have:\n\n\tn*x + n*b > n*x + n*24 + 2*b + 1024\n\n\tn*b - 2*b > n*24 + 1024\n\n\t(n - 2)*b > n*24 + 1024\n\nSo given this we need at least 3 objects for the whole to use more space \nthan a pack, and only if b is greater than 1096.  Since a common block \nsize is 4096 then the value of b is likely to converge towards 2048. 3 \nobjects is also where more directory entries are used over an constant \nof 2 for a pack (assuming that both objects would end up with the same \nfirst 2 bytes of hash which is overly optimistic).  So 3 should be the \noptimal number of objects for not exploding a pack.  And of course \nlarger packs are likely to take even less space due to delta compression \nkicking in.\n\nThis is why I think the current default treshold should be 3 instead of \nthe insane value of 5000.  But since it feels a bit odd to go from 5000 \nto 3 I setled on 10.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n\n---\n\nSidenote: I think it is OK for pushes to _not_ use thin packs.  When \nnot exploding thin packs, they must be completed by adding objects \ncreating duplicates and using more disk space.  The penalty for not \nusing thin packs is a slight increase in bandwidth for push operations, \nbut since pushes are normally much less frequent than fetches it seems \nOK to penalize the push a bit for a better disk usage on servers.\n\ndiff --git a/receive-pack.c b/receive-pack.c\nindex d62ed5b..9140312 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -11,7 +11,7 @@\n static const char receive_pack_usage[] = \"git-receive-pack <git-dir>\";\n \n static int deny_non_fast_forwards = 0;\n-static int unpack_limit = 5000;\n+static int unpack_limit = 10;\n static int report_status;\n \n"},{"id":"295313","messageId":"7vejrcy860.fsf@assigned-by-dhcp.cox.net","threadId":"43298","inReplyTo":"Pine.LNX.4.64.0612061420410.2630@xanadu.home","subject":"Re: [PATCH] change the unpack limit threshold to a saner value","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-06T22:24:39Z","receivedAt":"2006-12-06T22:24:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> Let's assume the average object size is x. Given n objects, the needed \n> storage size is n*(x + b), where b is the average wasted block size on \n> disk.\n> ...\n> This is why I think the current default treshold should be 3 instead of \n> the insane value of 5000.  But since it feels a bit odd to go from 5000 \n> to 3 I setled on 10.\n\nI see you are optimizing for disk footprint, and this will\nresult in tons of tiny packs left between \"repack -a\".\n\nI have not benched it yet, but the runtime pack handling code\nwas written assuming we have only a handful of big packs; I\nsuspect this change would affect the performance at runtime in\nquite a bad way.\n\n"},{"id":"296278","messageId":"Pine.LNX.4.64.0612061915240.2630@xanadu.home","threadId":"43298","inReplyTo":"7vejrcy860.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] change the unpack limit threshold to a saner value","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-12-07T00:19:41Z","receivedAt":"2006-12-07T00:19:41Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 6 Dec 2006, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@cam.org> writes:\n> \n> > Let's assume the average object size is x. Given n objects, the needed \n> > storage size is n*(x + b), where b is the average wasted block size on \n> > disk.\n> > ...\n> > This is why I think the current default treshold should be 3 instead of \n> > the insane value of 5000.  But since it feels a bit odd to go from 5000 \n> > to 3 I setled on 10.\n> \n> I see you are optimizing for disk footprint, and this will\n> result in tons of tiny packs left between \"repack -a\".\n\nDepends how often i.e. how big pushes are, and how often you repack.\n\n> I have not benched it yet, but the runtime pack handling code\n> was written assuming we have only a handful of big packs; I\n> suspect this change would affect the performance at runtime in\n> quite a bad way.\n\nPossibly.\n\nStill a treshold of 5000 is way too large IMHO.\n\n\n"},{"id":"297881","messageId":"Pine.LNX.4.64.0612061700120.3542@woody.osdl.org","threadId":"43298","inReplyTo":"Pine.LNX.4.64.0612061420410.2630@xanadu.home","subject":"Re: [PATCH] change the unpack limit treshold to a saner value","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-12-07T01:08:12Z","receivedAt":"2006-12-07T01:08:12Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Dec 2006, Nicolas Pitre wrote:\n> \n> This is why I think the current default treshold should be 3 instead of \n> the insane value of 5000.  But since it feels a bit odd to go from 5000 \n> to 3 I setled on 10.\n\nDefinitely not.\n\nWe have a much easier time handling many loose packed objects than many \npack-files. For many reasons, but two really obvious ones:\n\n - pack-file indexes get read in on startup, and we maintain an explicit \n   list of them. Having lots of pack-files adds overhead that doesn't \n   exist for lots of loose objects.\n\n - loose files are spread out over 256 subdirectories to make lookup \n   easier, packfiles are not (and always create an index file too).\n\nSo in general, as a trivial heuristic, you probably want about 512 times \nas many loose objects as you want pack-files, i fonly because of the \nlatter issue, because you can much more easily handle lots of loose \nobjects than lots of pack-files. So it's _not_ a factor of 3. Or even 10.\n\nBut since there _is_ reason to do pack-files too, and since using too big \na value means that you never end up keeping a pack-file _at_all_ if you \npull often, I'd suggest that rather than use a limit of 512 you go for \nsomething like 100-200 objects as the threshold (of course, the proper one \nwould depend on the distribution of the size of your pack-files, but I'll \njust hand-wave and say that together with occasional re-packing, something \nin that range is _generally_ going to be a good idea).\n\n"},{"id":"298593","messageId":"Pine.LNX.4.64.0612062213500.2630@xanadu.home","threadId":"43298","inReplyTo":"Pine.LNX.4.64.0612061700120.3542@woody.osdl.org","subject":"Re: [PATCH] change the unpack limit treshold to a saner value","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-12-07T03:24:14Z","receivedAt":"2006-12-07T03:24:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 6 Dec 2006, Linus Torvalds wrote:\n\n> We have a much easier time handling many loose packed objects than many \n> pack-files. For many reasons, but two really obvious ones:\n> \n>  - pack-file indexes get read in on startup, and we maintain an explicit \n>    list of them. Having lots of pack-files adds overhead that doesn't \n>    exist for lots of loose objects.\n> \n>  - loose files are spread out over 256 subdirectories to make lookup \n>    easier, packfiles are not (and always create an index file too).\n> \n> So in general, as a trivial heuristic, you probably want about 512 times \n> as many loose objects as you want pack-files, i fonly because of the \n> latter issue, because you can much more easily handle lots of loose \n> objects than lots of pack-files. So it's _not_ a factor of 3. Or even 10.\n> \n> But since there _is_ reason to do pack-files too, and since using too big \n> a value means that you never end up keeping a pack-file _at_all_ if you \n> pull often, I'd suggest that rather than use a limit of 512 you go for \n> something like 100-200 objects as the threshold (of course, the proper one \n> would depend on the distribution of the size of your pack-files, but I'll \n> just hand-wave and say that together with occasional re-packing, something \n> in that range is _generally_ going to be a good idea).\n\nNote that this setting is currently observed for pushes not pulls.\nOn the pull side you currentli need to provide -k for not exploding \npacks.\n\nSo the question is what number of objects on average do pushes have?  If \nmost pushes are below the treshold this is not going to be really \nuseful.\n\nAnd I think 5000 is definitely way too high.  10 might be too small \nindeed.  100 is maybe a good default to try out.\n\n\n"},{"id":"295589","messageId":"Pine.LNX.4.64.0612061935250.3615@woody.osdl.org","threadId":"43298","inReplyTo":"Pine.LNX.4.64.0612062213500.2630@xanadu.home","subject":"Re: [PATCH] change the unpack limit treshold to a saner value","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-12-07T03:39:35Z","receivedAt":"2006-12-07T03:39:35Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Dec 2006, Nicolas Pitre wrote:\n> \n> Note that this setting is currently observed for pushes not pulls.\n> On the pull side you currentli need to provide -k for not exploding \n> packs.\n\nSo noted.\n\n> So the question is what number of objects on average do pushes have?  If \n> most pushes are below the treshold this is not going to be really \n> useful.\n\nIt will depend a lot on the project, and \"where\" in the project you are.\n\nFor example, for most end developers, the \"push\" is likely going to be a \nfew commits (say, a days work). Probably on the order of a few tens to \nmaybe a few hundred objects. It's actually hard to create a pack with less \nthan ten objects if you have a few directory levels (a single small commit \nin the kernel is usually 5-7 objects: commit + 2-3 levels of directory + a \ncouple of blobs).\n\nFor me, as I pull a big merge and push it out, a push can easily be in the \nthousands of objects, just because I merged other peoples combined work \nover several weeks.\n\nAnd for a \"mirror\" server, it will depend on the granularity of the \nmirroring.\n\n> And I think 5000 is definitely way too high.  10 might be too small \n> indeed.  100 is maybe a good default to try out.\n\nI think 100 is a nice round number for humans. Worth trying.\n\n"},{"id":"294262","messageId":"Pine.LNX.4.64.0612062244090.2630@xanadu.home","threadId":"43298","inReplyTo":"Pine.LNX.4.64.0612061420410.2630@xanadu.home","subject":"[PATCH take 2] change the unpack limit treshold to a saner value","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-12-07T04:01:00Z","receivedAt":"2006-12-07T04:01:00Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Currently the treshold is 5000.  The likelihood of this value to ever be \ncrossed for a single push is really small making it not really useful.\n\nThe optimal treshold for a pure space saving on a filesystem with 4kb \nblocks is 3.  However this is likely to create many small packs \nconcentrating a large number of files in a single directory compared to \nthe same objects which are spread over 256 directories when loose.  This \nmeans we would need 512 objects per pack on average to approximagte the \nsame directory cost (a pack has 2 files because of the index).\n\nBut 512 is a really high value just like 5000 since most pushes are \nunlikely to have that many objects.  So let's try with a value of 100 \nwhich should have a good balance between small pushes going to be \nexploded into loose objects and large pushes kept as whole packs.\n\nThis is not a replacement for periodic repacks of course.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n\n---\n\ndiff --git a/receive-pack.c b/receive-pack.c\nindex d62ed5b..9140312 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -11,7 +11,7 @@\n static const char receive_pack_usage[] = \"git-receive-pack <git-dir>\";\n \n static int deny_non_fast_forwards = 0;\n-static int unpack_limit = 5000;\n+static int unpack_limit = 100;\n static int report_status;\n \n"},{"id":"296090","messageId":"20061207075956.GA22558@spearce.org","threadId":"43298","inReplyTo":"Pine.LNX.4.64.0612062244090.2630@xanadu.home","subject":"Re: [PATCH take 2] change the unpack limit treshold to a saner value","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-07T07:59:56Z","receivedAt":"2006-12-07T07:59:56Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nicolas Pitre <nico@cam.org> wrote:\n> Currently the treshold is 5000.  The likelihood of this value to ever be \n> crossed for a single push is really small making it not really useful.\n\nI started that out at 5000 because it was a reasonably safe\nthreshold.  Users who didn't explicitly lower this value didn't\nenable the feature.  But it was still useful on initial pushes into\na brand new repository when the project was very large.  For example\npushing git.git into a bare repository *sucked* before this change.\n\nYea, 5000 is probably too high.  Nice to see it drop.\n\n-- \n"}]}