{"thread":{"id":"50956","subject":"Resolving deltas dominates clone time","startedAt":"2019-04-19T21:47:26Z","lastAt":"2019-04-30T22:08:07Z","messageCount":26,"participants":["Martin Fick","Jeff King","Ævar Arnfjörð Bjarmason","Junio C Hamano","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"374167","messageId":"259296914.jpyqiltySj@mfick-lnx","threadId":"50956","inReplyTo":null,"subject":"Resolving deltas dominates clone time","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2019-04-19T21:47:22Z","receivedAt":"2019-04-19T21:47:26Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"We have a serious performance problem with one of our large repos. The repo is \nour internal version of the android platform/manifest project. Our repo after \nrunning a clean \"repack -A -d -F\" is close to 8G in size, has over 700K refs, \nand it has over 8M objects. The repo takes around 40min to clone locally (same \ndisk to same disk) using git 1.8.2.1 on a high end machine (56 processors, \n128GB RAM)! It takes around 10mins before getting to the resolving deltas \nphase which then takes most of the rest of the time.\n\nWhile this is a fairly large repo, a straight cp -r of the repo takes less \nthan 2mins, so I would expect a clone to be on the same order of magnitude in \ntime. For perspective, I have a kernel/msm repo with a third of the ref count \nand double the object count which takes only around 20mins to clone on the \nsame machine (still slower than I would like).\n\nI mention 1.8.2.1 because we have many old machines which need this. However, \nI also tested this with git v2.18 and it actually is much slower even \n(~140mins).\n\nReading the advice on the net, people seem to think that repacking with \nshorter delta-chains would help improve this. I have not had any success with \nthis yet.\n\nI have been thinking about this problem, and I suspect that this compute time \nis actually spent doing SHA1 calculations, is that possible? Some basic back \nof the envelope math and scripting seems to show that the repo may actually \ncontain about 2TB of data if you add up the size of all the objects in the \nrepo. Some quick research on the net seems to indicate that we might be able \nto expect something around 500MB/s throughput on computing SHA1s, does that \nseem reasonable? If I really have 2TB of data, should it then take around \n66mins to get the SHA1s for all that data? Could my repo clone time really be \ndominated by SHA1 math?\n\nAny advice on how to speed up cloning this repo, or what to pursue more \nin my investigation?\n\nThanks,\n\n-Martin\n\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n\n"},{"id":"374171","messageId":"20190420035825.GB3559@sigill.intra.peff.net","threadId":"50956","inReplyTo":"259296914.jpyqiltySj@mfick-lnx","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-20T03:58:25Z","receivedAt":"2019-04-20T03:58:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 19, 2019 at 03:47:22PM -0600, Martin Fick wrote:\n\n> I have been thinking about this problem, and I suspect that this compute time \n> is actually spent doing SHA1 calculations, is that possible? Some basic back \n> of the envelope math and scripting seems to show that the repo may actually \n> contain about 2TB of data if you add up the size of all the objects in the \n> repo. Some quick research on the net seems to indicate that we might be able \n> to expect something around 500MB/s throughput on computing SHA1s, does that \n> seem reasonable? If I really have 2TB of data, should it then take around \n> 66mins to get the SHA1s for all that data? Could my repo clone time really be \n> dominated by SHA1 math?\n\nThat sounds about right, actually. 8GB to 2TB is a compression ratio of\n250:1. That's bigger than I've seen, but I get 51:1 in the kernel.\n\nTry this (with a recent version of git; your v1.8.2.1 won't have\n--batch-all-objects):\n\n  # count the on-disk size of all objects\n  git cat-file --batch-all-objects --batch-check='%(objectsize) %(objectsize:disk)' |\n  perl -alne '\n    $repo += $F[0];\n    $disk += $F[1];\n    END { print \"$repo / $disk = \", $repo/$disk }\n  '\n\n250:1 isn't inconceivable if you have large blobs which have small\nchanges to them (and at 8GB for 8 million objects, you probably do have\nsome larger blobs, since the kernel is about 1/8th the size for the same\nnumber of objects).\n\nSo yes, if you really do have to hash 2TB of data, that's going to take\na while. \"openssl speed\" on my machine gives per-second speeds of:\n\ntype             16 bytes     64 bytes    256 bytes   1024 bytes   8192 bytes  16384 bytes\nsha1            135340.73k   337086.10k   677821.10k   909513.73k  1007528.62k  1016916.65k\n\nSo it's faster on bigger chunks, but yeah 500-1000MB/s seems like about\nthe best you're going to do. And...\n\n> I mention 1.8.2.1 because we have many old machines which need this. However, \n> I also tested this with git v2.18 and it actually is much slower even \n> (~140mins).\n\nI think v2.18 will have the collision-detecting sha1 on by default,\nwhich is slower. Building with OPENSSL_SHA1 should be the fastest (and\nare those numbers above). Git's internal (but not collision detecting)\nBLK_SHA1 is somewhere in the middle.\n\n> Any advice on how to speed up cloning this repo, or what to pursue more \n> in my investigation?\n\nIf you don't mind losing the collision-detection, using openssl's sha1\nmight help. The delta resolution should be threaded, too. So in _theory_\nyou're using 66 minutes of CPU time, but that should only take 1-2\nminutes on your 56-core machine. I don't know at what point you'd run\ninto lock contention, though. The locking there is quite coarse.\n\nWe also hash non-deltas while we're receiving them over the network.\nThat's accounted for in the \"receiving pack\" part of the progress meter.\nIf the time looks to be going to \"resolving deltas\", then that should\nall be threaded.\n\nIf you want to replay the slow part, it should just be index-pack. So\nsomething like (with $old as a fresh clone of the repo):\n\n  git init --bare new-repo.git\n  cd new-repo.git\n  perf record git index-pack -v --stdin <$old/.git/objects/pack/pack-*.pack\n  perf report\n\nshould show you where the time is going (substitute perf with whatever\nprofiling tool you like).\n\nAs far as avoiding that work altogether, there aren't a lot of options.\nGit clients do not trust the server, so the server sends only the raw\ndata, and the client is responsible for computing the object ids. The\nonly exception is a local filesystem clone, which will blindly copy or\nhardlink the .pack and .idx files from the source.\n\nIn theory there could be a protocol extension to let the client say \"I\ntrust you, please send me the matching .idx that goes with this pack,\nand I'll assume there was no bitrot nor trickery on your part\". I\ndon't recall anybody ever discussing such a patch in the past, but I\nthink Microsoft's VFS for Git project that backs development on Windows\nmight do similar trickery under the hood.\n\n-Peff\n"},{"id":"374173","messageId":"874l6tayzz.fsf@evledraar.gmail.com","threadId":"50956","inReplyTo":"20190420035825.GB3559@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-20T07:59:12Z","receivedAt":"2019-04-20T07:59:19Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Apr 20 2019, Jeff King wrote:\n\n> On Fri, Apr 19, 2019 at 03:47:22PM -0600, Martin Fick wrote:\n>\n>> I have been thinking about this problem, and I suspect that this compute time\n>> is actually spent doing SHA1 calculations, is that possible? Some basic back\n>> of the envelope math and scripting seems to show that the repo may actually\n>> contain about 2TB of data if you add up the size of all the objects in the\n>> repo. Some quick research on the net seems to indicate that we might be able\n>> to expect something around 500MB/s throughput on computing SHA1s, does that\n>> seem reasonable? If I really have 2TB of data, should it then take around\n>> 66mins to get the SHA1s for all that data? Could my repo clone time really be\n>> dominated by SHA1 math?\n>\n> That sounds about right, actually. 8GB to 2TB is a compression ratio of\n> 250:1. That's bigger than I've seen, but I get 51:1 in the kernel.\n>\n> Try this (with a recent version of git; your v1.8.2.1 won't have\n> --batch-all-objects):\n>\n>   # count the on-disk size of all objects\n>   git cat-file --batch-all-objects --batch-check='%(objectsize) %(objectsize:disk)' |\n>   perl -alne '\n>     $repo += $F[0];\n>     $disk += $F[1];\n>     END { print \"$repo / $disk = \", $repo/$disk }\n>   '\n>\n> 250:1 isn't inconceivable if you have large blobs which have small\n> changes to them (and at 8GB for 8 million objects, you probably do have\n> some larger blobs, since the kernel is about 1/8th the size for the same\n> number of objects).\n>\n> So yes, if you really do have to hash 2TB of data, that's going to take\n> a while. \"openssl speed\" on my machine gives per-second speeds of:\n>\n> type             16 bytes     64 bytes    256 bytes   1024 bytes   8192 bytes  16384 bytes\n> sha1            135340.73k   337086.10k   677821.10k   909513.73k  1007528.62k  1016916.65k\n>\n> So it's faster on bigger chunks, but yeah 500-1000MB/s seems like about\n> the best you're going to do. And...\n>\n>> I mention 1.8.2.1 because we have many old machines which need this. However,\n>> I also tested this with git v2.18 and it actually is much slower even\n>> (~140mins).\n>\n> I think v2.18 will have the collision-detecting sha1 on by default,\n> which is slower. Building with OPENSSL_SHA1 should be the fastest (and\n> are those numbers above). Git's internal (but not collision detecting)\n> BLK_SHA1 is somewhere in the middle.\n>\n>> Any advice on how to speed up cloning this repo, or what to pursue more\n>> in my investigation?\n>\n> If you don't mind losing the collision-detection, using openssl's sha1\n> might help. The delta resolution should be threaded, too. So in _theory_\n> you're using 66 minutes of CPU time, but that should only take 1-2\n> minutes on your 56-core machine. I don't know at what point you'd run\n> into lock contention, though. The locking there is quite coarse.\n\nThere's also my (been meaning to re-roll)\nhttps://public-inbox.org/git/20181113201910.11518-1-avarab@gmail.com/\n*that* part of the SHA-1 checking is part of what's going on here. It'll\nhelp a *tiny* bit, but of course is part of the \"trust remote\" risk\nmanagement...\n\n> We also hash non-deltas while we're receiving them over the network.\n> That's accounted for in the \"receiving pack\" part of the progress meter.\n> If the time looks to be going to \"resolving deltas\", then that should\n> all be threaded.\n>\n> If you want to replay the slow part, it should just be index-pack. So\n> something like (with $old as a fresh clone of the repo):\n>\n>   git init --bare new-repo.git\n>   cd new-repo.git\n>   perf record git index-pack -v --stdin <$old/.git/objects/pack/pack-*.pack\n>   perf report\n>\n> should show you where the time is going (substitute perf with whatever\n> profiling tool you like).\n>\n> As far as avoiding that work altogether, there aren't a lot of options.\n> Git clients do not trust the server, so the server sends only the raw\n> data, and the client is responsible for computing the object ids. The\n> only exception is a local filesystem clone, which will blindly copy or\n> hardlink the .pack and .idx files from the source.\n>\n> In theory there could be a protocol extension to let the client say \"I\n> trust you, please send me the matching .idx that goes with this pack,\n> and I'll assume there was no bitrot nor trickery on your part\". I\n> don't recall anybody ever discussing such a patch in the past, but I\n> think Microsoft's VFS for Git project that backs development on Windows\n> might do similar trickery under the hood.\n\nI started to write:\n\n    I wonder if there's room for some tacit client/server cooperation\n    without such a protocol change.\n\n    E.g. the server sending over a pack constructed in such a way that\n    everything required for a checkout is at the beginning of the\n    data. Now we implicitly tend to do it mostly the other way around\n    for delta optimization purposes.\n\n    That would allow a smart client in a hurry to index-pack it as they\n    go along, and as soon as they have enough to check out HEAD return\n    to the client, and continue the rest in the background\n\nBut realized I was just starting to describe something like 'clone\n--depth=1' followed by a 'fetch --unshallow' in the background, except\nthat would work better (if you did \"just the tip\" naïvely you'd get\n'missing object' on e.g. 'git log', with that ad-hoc hack we'd need to\nwrite out two packs etc...).\n\n    $ rm -rf /tmp/git; time git clone --depth=1 https://chromium.googlesource.com/chromium/src /tmp/git; time git -C /tmp/git fetch --unshallow\n    Cloning into '/tmp/git'...\n    remote: Counting objects: 304839, done\n    remote: Finding sources: 100% (304839/304839)\n    remote: Total 304839 (delta 70483), reused 204837 (delta 70483)\n    Receiving objects: 100% (304839/304839), 1.48 GiB | 19.87 MiB/s, done.\n    Resolving deltas: 100% (70483/70483), done.\n    Checking out files: 100% (302768/302768), done.\n\n    real    2m10.223s\n    user    1m2.434s\n    sys     0m15.564s\n    [not waiting for that second bit, but it'll take ages...]\n\nI think just having a clone mode that did that for you might scratch a\nlot of people's itch. I.e. \"I want full history, but mainly want a\ncheckout right away, so background the full clone\".\n\nBut at this point I'm just starting to describe some shoddy version of\nDocumentation/technical/partial-clone.txt :), OTOH there's no \"narrow\nclone and fleshen right away\" option.\n\nOn protocol extensions: Just having a way to \"wget\" the corresponding\n*.idx file from the server would be great, and reduce clone times by a\nlot. There's the risk of trusting the server, but most people's use-case\nis going to be pushing right back to the same server, which'll be doing\na full validation.\n\nWe could also defer that validation instead of skipping it. E.g. wget\n*.{pack,idx} followed by a 'fsck' in the background. I've sometimes\nwanted that anyway, i.e. \"fsck --auto\" similar to \"gc --auto\"\nperiodically to detect repository bitflips.\n\nOr, do some \"narrow\" validation of such an *.idx file right\naway. E.g. for all the trees/blobs required for the current checkout,\nand background the rest.\n"},{"id":"374250","messageId":"20190422155716.GA9680@sigill.intra.peff.net","threadId":"50956","inReplyTo":"874l6tayzz.fsf@evledraar.gmail.com","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-22T15:57:16Z","receivedAt":"2019-04-22T15:57:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 20, 2019 at 09:59:12AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > If you don't mind losing the collision-detection, using openssl's sha1\n> > might help. The delta resolution should be threaded, too. So in _theory_\n> > you're using 66 minutes of CPU time, but that should only take 1-2\n> > minutes on your 56-core machine. I don't know at what point you'd run\n> > into lock contention, though. The locking there is quite coarse.\n> \n> There's also my (been meaning to re-roll)\n> https://public-inbox.org/git/20181113201910.11518-1-avarab@gmail.com/\n> *that* part of the SHA-1 checking is part of what's going on here. It'll\n> help a *tiny* bit, but of course is part of the \"trust remote\" risk\n> management...\n\nI think we're talking about two different collision detections, and your\npatch wouldn't help at all here.\n\nYour patch is optionally removing the \"woah, we got an object with a\nduplicate sha1, let's check that the bytes are the same in both copies\"\ncheck. But Martin's problem is a clone, so we wouldn't have any existing\nobjects to duplicate in the first place.\n\nThe problem in his case is literally just that the actual SHA-1 is\nexpensive, and that can be helped by using the optimized openssl\nimplementation rather than the sha1dc (which checks not collisions with\nobjects we _have_, but evidence of somebody trying to exploit weaknesses\nin sha1).\n\nOne thing we could do to make that easier is a run-time flag to switch\nbetween sha1dc and a faster implementation (either openssl or blk_sha1,\ndepending on the build). That would let you flip the \"trust\" bit per\noperation, rather than having it baked into your build.\n\n(Note that the oft-discussed \"use a faster sha1 implementation for\nchecksums, but sha1dc for object hashing\" idea would not help here,\nbecause these really are object hashes whose time is dominating. We have\nto checksum 8GB of raw packfile but 2TB of object data).\n\n> I started to write:\n> \n>     I wonder if there's room for some tacit client/server cooperation\n>     without such a protocol change.\n> \n>     E.g. the server sending over a pack constructed in such a way that\n>     everything required for a checkout is at the beginning of the\n>     data. Now we implicitly tend to do it mostly the other way around\n>     for delta optimization purposes.\n> \n>     That would allow a smart client in a hurry to index-pack it as they\n>     go along, and as soon as they have enough to check out HEAD return\n>     to the client, and continue the rest in the background\n\nInteresting idea. You're not reducing the total client effort, but\nyou're improving latency of getting the user to a checkout. Of course\nthat doesn't help if they want to run \"git log\" as their first\noperation. ;)\n\n> But realized I was just starting to describe something like 'clone\n> --depth=1' followed by a 'fetch --unshallow' in the background, except\n> that would work better (if you did \"just the tip\" naïvely you'd get\n> 'missing object' on e.g. 'git log', with that ad-hoc hack we'd need to\n> write out two packs etc...).\n\nRight, that would work. I will note one thing, though: the total time to\ndo a 1-depth clone followed by an unshallow is probably much higher than\ndoing the whole clone as one unit, for two reasons:\n\n  1. The server won't use reachability bitmaps when serving the\n     follow-up fetch (because shallowness invalidates the reachability\n     data they're caching), so it will spend much more time in the\n     \"Counting objects\" phase.\n\n  2. The server has to throw away some deltas. Imagine version X of a\n     file in the tip commit is stored as a delta against version Y in\n     that commit's parent. The initial clone has to throw away the\n     on-disk delta of X and send you the whole object (because you are\n     not requesting Y at all). And then in the follow-up fetch, it must\n     either send you Y as a base object (wasting bandwidth), or it must\n     on-the-fly generate a delta from Y to X (wasting CPU).\n\n> But at this point I'm just starting to describe some shoddy version of\n> Documentation/technical/partial-clone.txt :), OTOH there's no \"narrow\n> clone and fleshen right away\" option.\n\nYes. And partial-clone suffers from the problems above to an even\ngreater extent. ;)\n\n> On protocol extensions: Just having a way to \"wget\" the corresponding\n> *.idx file from the server would be great, and reduce clone times by a\n> lot. There's the risk of trusting the server, but most people's use-case\n> is going to be pushing right back to the same server, which'll be doing\n> a full validation.\n\nOne tricky thing is that the server may be handing you a bespoke .pack\nfile. There is no matching \".idx\" at all, neither in-memory nor on disk.\nAnd you would not want the whole on-disk .pack/.idx pair from a site\nlike GitHub, where there are objects from many forks.\n\nSo in general, I think you'd need some cooperation from the server side\nto ask it to generate and send the .idx that matches the .pack it is\nsending you. Or even if not the .idx format itself, some stable list of\nsha1s that you could use to reproduce it without hashing each\nuncompressed byte yourself. This could even be stuffed into the pack\nformat and stripped out by the receiving index-pack (i.e., each entry is\nprefixed with \"and by the way, here is my sha1...\").\n\n> We could also defer that validation instead of skipping it. E.g. wget\n> *.{pack,idx} followed by a 'fsck' in the background. I've sometimes\n> wanted that anyway, i.e. \"fsck --auto\" similar to \"gc --auto\"\n> periodically to detect repository bitflips.\n> \n> Or, do some \"narrow\" validation of such an *.idx file right\n> away. E.g. for all the trees/blobs required for the current checkout,\n> and background the rest.\n\nThe \"do we have all of the objects we need\" is already separate from\n\"figure out the sha1 of each object\", so I think you'd get that\nnaturally if you just took in an untrusted .idx (it also demonstrates\nthat any .idx cost is really focusing on blobs, because the \"do we have\nall objects\" check is going to decompress every commit and tree in the\nrepo anyway).\n\n-Peff\n"},{"id":"374258","messageId":"874l6pudg4.fsf@evledraar.gmail.com","threadId":"50956","inReplyTo":"20190422155716.GA9680@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-22T18:01:15Z","receivedAt":"2019-04-22T18:01:25Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Apr 22 2019, Jeff King wrote:\n\n> On Sat, Apr 20, 2019 at 09:59:12AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > If you don't mind losing the collision-detection, using openssl's sha1\n>> > might help. The delta resolution should be threaded, too. So in _theory_\n>> > you're using 66 minutes of CPU time, but that should only take 1-2\n>> > minutes on your 56-core machine. I don't know at what point you'd run\n>> > into lock contention, though. The locking there is quite coarse.\n>>\n>> There's also my (been meaning to re-roll)\n>> https://public-inbox.org/git/20181113201910.11518-1-avarab@gmail.com/\n>> *that* part of the SHA-1 checking is part of what's going on here. It'll\n>> help a *tiny* bit, but of course is part of the \"trust remote\" risk\n>> management...\n>\n> I think we're talking about two different collision detections, and your\n> patch wouldn't help at all here.\n>\n> Your patch is optionally removing the \"woah, we got an object with a\n> duplicate sha1, let's check that the bytes are the same in both copies\"\n> check. But Martin's problem is a clone, so we wouldn't have any existing\n> objects to duplicate in the first place.\n\nRight, but we do anyway, as reported by Geert at @amazon.com preceding\nthat patch of mine. But it is 99.99% irrelevant to *performance* in this\ncase after the loose object cache you added (but before that could make\nall the difference depending on the FS).\n\nI just mentioned it to plant a flag on another bit of the code where\nindex-pack in general has certain paranoias/validation the user might be\nwilling to optionally drop just at \"clone\" time.\n\n> The problem in his case is literally just that the actual SHA-1 is\n> expensive, and that can be helped by using the optimized openssl\n> implementation rather than the sha1dc (which checks not collisions with\n> objects we _have_, but evidence of somebody trying to exploit weaknesses\n> in sha1).\n>\n> One thing we could do to make that easier is a run-time flag to switch\n> between sha1dc and a faster implementation (either openssl or blk_sha1,\n> depending on the build). That would let you flip the \"trust\" bit per\n> operation, rather than having it baked into your build.\n\nYeah, this would be neat.\n\n> (Note that the oft-discussed \"use a faster sha1 implementation for\n> checksums, but sha1dc for object hashing\" idea would not help here,\n> because these really are object hashes whose time is dominating. We have\n> to checksum 8GB of raw packfile but 2TB of object data).\n>\n>> I started to write:\n>>\n>>     I wonder if there's room for some tacit client/server cooperation\n>>     without such a protocol change.\n>>\n>>     E.g. the server sending over a pack constructed in such a way that\n>>     everything required for a checkout is at the beginning of the\n>>     data. Now we implicitly tend to do it mostly the other way around\n>>     for delta optimization purposes.\n>>\n>>     That would allow a smart client in a hurry to index-pack it as they\n>>     go along, and as soon as they have enough to check out HEAD return\n>>     to the client, and continue the rest in the background\n>\n> Interesting idea. You're not reducing the total client effort, but\n> you're improving latency of getting the user to a checkout. Of course\n> that doesn't help if they want to run \"git log\" as their first\n> operation. ;)\n>\n>> But realized I was just starting to describe something like 'clone\n>> --depth=1' followed by a 'fetch --unshallow' in the background, except\n>> that would work better (if you did \"just the tip\" naïvely you'd get\n>> 'missing object' on e.g. 'git log', with that ad-hoc hack we'd need to\n>> write out two packs etc...).\n>\n> Right, that would work. I will note one thing, though: the total time to\n> do a 1-depth clone followed by an unshallow is probably much higher than\n> doing the whole clone as one unit, for two reasons:\n\nIndeed. The hypothesis is that the user doesn't really care about the\nclone-time, but the clone-to-repo-mostly-usable time.\n\n>   1. The server won't use reachability bitmaps when serving the\n>      follow-up fetch (because shallowness invalidates the reachability\n>      data they're caching), so it will spend much more time in the\n>      \"Counting objects\" phase.\n>\n>   2. The server has to throw away some deltas. Imagine version X of a\n>      file in the tip commit is stored as a delta against version Y in\n>      that commit's parent. The initial clone has to throw away the\n>      on-disk delta of X and send you the whole object (because you are\n>      not requesting Y at all). And then in the follow-up fetch, it must\n>      either send you Y as a base object (wasting bandwidth), or it must\n>      on-the-fly generate a delta from Y to X (wasting CPU).\n>\n>> But at this point I'm just starting to describe some shoddy version of\n>> Documentation/technical/partial-clone.txt :), OTOH there's no \"narrow\n>> clone and fleshen right away\" option.\n>\n> Yes. And partial-clone suffers from the problems above to an even\n> greater extent. ;)\n>\n>> On protocol extensions: Just having a way to \"wget\" the corresponding\n>> *.idx file from the server would be great, and reduce clone times by a\n>> lot. There's the risk of trusting the server, but most people's use-case\n>> is going to be pushing right back to the same server, which'll be doing\n>> a full validation.\n>\n> One tricky thing is that the server may be handing you a bespoke .pack\n> file. There is no matching \".idx\" at all, neither in-memory nor on disk.\n> And you would not want the whole on-disk .pack/.idx pair from a site\n> like GitHub, where there are objects from many forks.\n>\n> So in general, I think you'd need some cooperation from the server side\n> to ask it to generate and send the .idx that matches the .pack it is\n> sending you. Or even if not the .idx format itself, some stable list of\n> sha1s that you could use to reproduce it without hashing each\n> uncompressed byte yourself.\n\nYeah, depending on how jt/fetch-cdn-offload is designed (see my\nhttps://public-inbox.org/git/87k1hv6eel.fsf@evledraar.gmail.com/) it\ncould be (ab)used to do this. I.e. you'd keep a \"base\" *.{pack,idx}\naround for such a purpose.\n\nSo in such a case you'd serve up that recent-enough *.{pack,idx} for the\nclient to \"wget\", and the client would then trust it (or not) and do the\nequivalent of a \"fetch\" from that point to be 100% up-to-date.\n\n> This could even be stuffed into the pack format and stripped out by\n> the receiving index-pack (i.e., each entry is prefixed with \"and by\n> the way, here is my sha1...\").\n\nThat would be really interesting. I.e. just having room for that (or\nanything else) in the pack format.\n\nI wonder if it could be added to the delta-chain in the current format\nas a nasty hack :)\n\n>> We could also defer that validation instead of skipping it. E.g. wget\n>> *.{pack,idx} followed by a 'fsck' in the background. I've sometimes\n>> wanted that anyway, i.e. \"fsck --auto\" similar to \"gc --auto\"\n>> periodically to detect repository bitflips.\n>>\n>> Or, do some \"narrow\" validation of such an *.idx file right\n>> away. E.g. for all the trees/blobs required for the current checkout,\n>> and background the rest.\n>\n> The \"do we have all of the objects we need\" is already separate from\n> \"figure out the sha1 of each object\", so I think you'd get that\n> naturally if you just took in an untrusted .idx (it also demonstrates\n> that any .idx cost is really focusing on blobs, because the \"do we have\n> all objects\" check is going to decompress every commit and tree in the\n> repo anyway).\n>\n> -Peff\n"},{"id":"374262","messageId":"20190422184329.GA20304@sigill.intra.peff.net","threadId":"50956","inReplyTo":"874l6pudg4.fsf@evledraar.gmail.com","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-22T18:43:29Z","receivedAt":"2019-04-22T18:43:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 22, 2019 at 08:01:15PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > Your patch is optionally removing the \"woah, we got an object with a\n> > duplicate sha1, let's check that the bytes are the same in both copies\"\n> > check. But Martin's problem is a clone, so we wouldn't have any existing\n> > objects to duplicate in the first place.\n> \n> Right, but we do anyway, as reported by Geert at @amazon.com preceding\n> that patch of mine. But it is 99.99% irrelevant to *performance* in this\n> case after the loose object cache you added (but before that could make\n> all the difference depending on the FS).\n\nI scratched my head at this a bit. If we don't have any other objects,\nthen what are we comparing against? But I think you mean that we have\nthe overhead of doing the object lookups to find that out. Yes, that can\nadd up if your filesystem has high latency, but I think in this case it\nis a drop in the bucket compared to dealing with the actual object data.\n\n> I just mentioned it to plant a flag on another bit of the code where\n> index-pack in general has certain paranoias/validation the user might be\n> willing to optionally drop just at \"clone\" time.\n\nYeah, I agree it may be worth pursuing independently. I just don't think\nit will help Martin's case in any noticeable way.\n\n> > Right, that would work. I will note one thing, though: the total time to\n> > do a 1-depth clone followed by an unshallow is probably much higher than\n> > doing the whole clone as one unit, for two reasons:\n> \n> Indeed. The hypothesis is that the user doesn't really care about the\n> clone-time, but the clone-to-repo-mostly-usable time.\n\nThere was a little bit of self-interest in there for me, too, as a\nserver operator. While it does add to the end-to-end time, most of the\nresource use for the shallow fetch gets put on the server. IOW, I don't\nthink we'd be happy to see clients doing this depth-1-and-then-unshallow\nstrategy for every clone.\n\n> > So in general, I think you'd need some cooperation from the server side\n> > to ask it to generate and send the .idx that matches the .pack it is\n> > sending you. Or even if not the .idx format itself, some stable list of\n> > sha1s that you could use to reproduce it without hashing each\n> > uncompressed byte yourself.\n> \n> Yeah, depending on how jt/fetch-cdn-offload is designed (see my\n> https://public-inbox.org/git/87k1hv6eel.fsf@evledraar.gmail.com/) it\n> could be (ab)used to do this. I.e. you'd keep a \"base\" *.{pack,idx}\n> around for such a purpose.\n> \n> So in such a case you'd serve up that recent-enough *.{pack,idx} for the\n> client to \"wget\", and the client would then trust it (or not) and do the\n> equivalent of a \"fetch\" from that point to be 100% up-to-date.\n\nI think it's sort of orthogonal. Either way you have to teach the client\nhow to get a .pack/.idx combo. Whether it learns to receive it inline\nfrom the first fetch, or whether it is taught to expect it from the\nout-of-band fetch, most of the challenge is the same.\n\n> > This could even be stuffed into the pack format and stripped out by\n> > the receiving index-pack (i.e., each entry is prefixed with \"and by\n> > the way, here is my sha1...\").\n> \n> That would be really interesting. I.e. just having room for that (or\n> anything else) in the pack format.\n> \n> I wonder if it could be added to the delta-chain in the current format\n> as a nasty hack :)\n\nThere's definitely not \"room\" in any sense of the word in the pack\nformat. :) However, as long as all parties agreed, we can stick whatever\nwe want into the on-the-wire format. So I was imagining something more\nlike:\n\n  1. pack-objects learns a --report-object-id option that sticks some\n     additional bytes before each object (in its simplest form,\n     $obj_hash bytes of id)\n\n  2. likewise, index-pack learns a --parse-object-id option to receive\n     it and skip hashing the object bytes\n\n  3. we get a new protocol capability, \"send-object-ids\". If the server\n     advertises and the client requests it, then both sides turn on the\n     appropriate option\n\nYou could even imagine generalizing it to \"--report-object-metadata\",\nand including 0 or more metadata packets before each object. With object\nid being one, but possibly other computable bits like \"generation\nnumber\" after that. I'm not convinced other metadata is worth the\nspace/time tradeoff, though. After all, this is stuff that the client\n_could_ generate and cache themselves, so you're trading off bandwidth\nto save the client from doing the computation.\n\nAnyway, food for thought. :)\n\n-Peff\n"},{"id":"374268","messageId":"16052712.dFCfNLlQnN@mfick-lnx","threadId":"50956","inReplyTo":"20190420035825.GB3559@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2019-04-22T20:21:40Z","receivedAt":"2019-04-22T20:21:45Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Friday, April 19, 2019 11:58:25 PM MDT Jeff King wrote:\n> On Fri, Apr 19, 2019 at 03:47:22PM -0600, Martin Fick wrote:\n> > I have been thinking about this problem, and I suspect that this compute\n> > time is actually spent doing SHA1 calculations, is that possible? Some\n> > basic back of the envelope math and scripting seems to show that the repo\n> > may actually contain about 2TB of data if you add up the size of all the\n> > objects in the repo. Some quick research on the net seems to indicate\n> > that we might be able to expect something around 500MB/s throughput on\n> > computing SHA1s, does that seem reasonable? If I really have 2TB of data,\n> > should it then take around 66mins to get the SHA1s for all that data?\n> > Could my repo clone time really be dominated by SHA1 math?\n> \n> That sounds about right, actually. 8GB to 2TB is a compression ratio of\n> 250:1. That's bigger than I've seen, but I get 51:1 in the kernel.\n> \n> Try this (with a recent version of git; your v1.8.2.1 won't have\n> --batch-all-objects):\n> \n>   # count the on-disk size of all objects\n>   git cat-file --batch-all-objects --batch-check='%(objectsize)\n> %(objectsize:disk)' | perl -alne '\n>     $repo += $F[0];\n>     $disk += $F[1];\n>     END { print \"$repo / $disk = \", $repo/$disk }\n>   '\n\nThis has been running for a few hours now, I will update you with results when \nits done.\n\n> 250:1 isn't inconceivable if you have large blobs which have small\n> changes to them (and at 8GB for 8 million objects, you probably do have\n> some larger blobs, since the kernel is about 1/8th the size for the same\n> number of objects).\n\nI think it's mostly xml files in the 1-10MB range.\n\n> So yes, if you really do have to hash 2TB of data, that's going to take\n> a while.\n\nI was hoping I was wrong. Unfortunately I sense that this is not likely \nsomething we can improve with a better algorithm. It seems like the best way \nto handle this long term is likely to use BUP's rolling hash splitting, it \nwould make this way better (assuming it made objects small enough). I think it \nis interesting that this approach might end up being effective for more than \njust large binary file repos. If I could get this repo into bup somehow, it \ncould potentially show us if this would drastically reduce the index-pack \ntime.\n\n> I think v2.18 will have the collision-detecting sha1 on by default,\n> which is slower.\n\nMakes sense.\n\n> If you don't mind losing the collision-detection, using openssl's sha1\n> might help. The delta resolution should be threaded, too. So in _theory_\n> you're using 66 minutes of CPU time, but that should only take 1-2\n> minutes on your 56-core machine. I don't know at what point you'd run\n> into lock contention, though. The locking there is quite coarse.\n\nI suspect at 3 threads, seems like the default?\n\nI am running some index packs to test the theory, I can tell you already that \nthe 56 thread versions was much slower, it took 397m25.622s. I am running a \nfew other tests also, but it will take a while to get an answer. Since things \ntake hours to test, I made a repo with a single branch (and the tags for that \nbranch) from this bigger repo using a git init/git fetch. The single branch \nrepo takes about 12s to clone, but it takes around 14s with 3 threads to run \nindex-pack, any ideas why it is slower than a clone?\n\nHere are some thread times for the single branch case:\n\n Threads  Time\n 56           49s\n 12           34s\n 5             20s\n 4             15s\n 3             14s\n 2             17\n 1             30\n\nSo 3 threads appears optimal in this case.\n\nPerhaps the locking can be improved here to make threading more effective?\n\n> We also hash non-deltas while we're receiving them over the network.\n> That's accounted for in the \"receiving pack\" part of the progress meter.\n> If the time looks to be going to \"resolving deltas\", then that should\n> all be threaded.\n\nWould it make sense to make the receiving pack time also threaded because I \nbelieve that time is still longer than the I/O time (2 or 3 times)?\n\n> If you want to replay the slow part, it should just be index-pack. So\n> something like (with $old as a fresh clone of the repo):\n> \n>   git init --bare new-repo.git\n>   cd new-repo.git\n>   perf record git index-pack -v --stdin <$old/.git/objects/pack/pack-*.pack\n>   perf report\n> \n> should show you where the time is going (substitute perf with whatever\n> profiling tool you like).\n\nI will work on profiling soon, but I wanted to give an update now.\n\nThanks for the great feedback,\n \n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n\n"},{"id":"374269","messageId":"20190422205653.GA30286@sigill.intra.peff.net","threadId":"50956","inReplyTo":"16052712.dFCfNLlQnN@mfick-lnx","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-22T20:56:54Z","receivedAt":"2019-04-22T20:56:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 22, 2019 at 02:21:40PM -0600, Martin Fick wrote:\n\n> > Try this (with a recent version of git; your v1.8.2.1 won't have\n> > --batch-all-objects):\n> > \n> >   # count the on-disk size of all objects\n> >   git cat-file --batch-all-objects --batch-check='%(objectsize)\n> > %(objectsize:disk)' | perl -alne '\n> >     $repo += $F[0];\n> >     $disk += $F[1];\n> >     END { print \"$repo / $disk = \", $repo/$disk }\n> >   '\n> \n> This has been running for a few hours now, I will update you with results when \n> its done.\n\nHours? I think something might be wrong. It takes 20s to run on\nlinux.git.\n\n> > 250:1 isn't inconceivable if you have large blobs which have small\n> > changes to them (and at 8GB for 8 million objects, you probably do have\n> > some larger blobs, since the kernel is about 1/8th the size for the same\n> > number of objects).\n> \n> I think it's mostly xml files in the 1-10MB range.\n\nYeah, I could believe that would do it, then. Take a 10MB file with a\nhundred 1K updates. That'd be 99*1K + 10MB to store, for almost 100:1\ncompression.\n\nThe key is really having objects where the size of change versus the\nfile size is small (so the marginal cost of each revision is small), and\nthen having lots of changes (to give you a big multiplier).\n\n> > So yes, if you really do have to hash 2TB of data, that's going to take\n> > a while.\n> \n> I was hoping I was wrong. Unfortunately I sense that this is not likely \n> something we can improve with a better algorithm. It seems like the best way \n> to handle this long term is likely to use BUP's rolling hash splitting, it \n> would make this way better (assuming it made objects small enough). I think it \n> is interesting that this approach might end up being effective for more than \n> just large binary file repos. If I could get this repo into bup somehow, it \n> could potentially show us if this would drastically reduce the index-pack \n> time.\n\nIt fundamentally can't help without changing Git's object model, since\nyou need to have complete hashes of all of those files. If you're\nproposing to do rolling hashes to split blobs into multiple objects that\nwould definitely work. But it wouldn't be compatible with Git anymore.\n\n> > If you don't mind losing the collision-detection, using openssl's sha1\n> > might help. The delta resolution should be threaded, too. So in _theory_\n> > you're using 66 minutes of CPU time, but that should only take 1-2\n> > minutes on your 56-core machine. I don't know at what point you'd run\n> > into lock contention, though. The locking there is quite coarse.\n> \n> I suspect at 3 threads, seems like the default?\n\nAh, right, I forgot we cap it at 3 (which was determined experimentally,\nand which we more or less attributed to lock contention as the\nbottleneck). I think you need to use $GIT_FORCE_THREADS to override it.\n\n> I am running some index packs to test the theory, I can tell you already that \n> the 56 thread versions was much slower, it took 397m25.622s. I am running a \n> few other tests also, but it will take a while to get an answer. Since things \n> take hours to test, I made a repo with a single branch (and the tags for that \n> branch) from this bigger repo using a git init/git fetch. The single branch \n> repo takes about 12s to clone, but it takes around 14s with 3 threads to run \n> index-pack, any ideas why it is slower than a clone?\n\nAre you running it in the same repo, or in another newly-created repo?\nOr alternatively, in a new repo but repeatedly running index-pack? After\nthe first run, that repo will have all of the objects. And so for each\nobject it sees, index-pack will say \"woah, we already had that one;\nlet's double check that they're byte for byte identical\" which carries\nextra overhead (and probably makes the lock contention way worse, too,\nbecause accessing existing objects just has one big coarse lock).\n\nSo definitely do something like:\n\n  for threads in 1 2 3 4 5 12 56; do\n\trm -rf repo.git\n\tgit init --bare repo.git\n\tGIT_FORCE_THREADS=$threads \\\n\t  git -C repo.git index-pack -v --stdin </path/to/pack\n  done\n\nto test.\n\n> Perhaps the locking can be improved here to make threading more effective?\n\nProbably, but easier said than done, of course.\n\n> > We also hash non-deltas while we're receiving them over the network.\n> > That's accounted for in the \"receiving pack\" part of the progress meter.\n> > If the time looks to be going to \"resolving deltas\", then that should\n> > all be threaded.\n> \n> Would it make sense to make the receiving pack time also threaded because I\n> believe that time is still longer than the I/O time (2 or 3 times)?\n\nIt's a lot harder to thread since we're eating the incoming bytes. And\nunless you're just coming from a local disk copy, the network is\ngenerally the bottleneck (and if you are coming from a local disk copy,\nthen consider doing a local clone which will avoid all of this hashing\nin the first place).\n\n-Peff\n"},{"id":"374270","messageId":"20190422210214.GA31079@sigill.intra.peff.net","threadId":"50956","inReplyTo":"20190422205653.GA30286@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-22T21:02:15Z","receivedAt":"2019-04-22T21:02:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 22, 2019 at 04:56:54PM -0400, Jeff King wrote:\n\n> > I suspect at 3 threads, seems like the default?\n> \n> Ah, right, I forgot we cap it at 3 (which was determined experimentally,\n> and which we more or less attributed to lock contention as the\n> bottleneck). I think you need to use $GIT_FORCE_THREADS to override it.\n\nAh, nevermind this. Using --threads will do what you expect. The cap at\n3 only applies when we've just picked the number of available CPUs as a\ndefault.\n\n-Peff\n"},{"id":"374271","messageId":"20190422211952.GA4728@sigill.intra.peff.net","threadId":"50956","inReplyTo":"20190422205653.GA30286@sigill.intra.peff.net","subject":"[PATCH] p5302: create the repo in each index-pack test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-22T21:19:52Z","receivedAt":"2019-04-22T21:19:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 22, 2019 at 04:56:53PM -0400, Jeff King wrote:\n\n> > I am running some index packs to test the theory, I can tell you already that \n> > the 56 thread versions was much slower, it took 397m25.622s. I am running a \n> > few other tests also, but it will take a while to get an answer. Since things \n> > take hours to test, I made a repo with a single branch (and the tags for that \n> > branch) from this bigger repo using a git init/git fetch. The single branch \n> > repo takes about 12s to clone, but it takes around 14s with 3 threads to run \n> > index-pack, any ideas why it is slower than a clone?\n> \n> Are you running it in the same repo, or in another newly-created repo?\n> Or alternatively, in a new repo but repeatedly running index-pack? After\n> the first run, that repo will have all of the objects. And so for each\n> object it sees, index-pack will say \"woah, we already had that one;\n> let's double check that they're byte for byte identical\" which carries\n> extra overhead (and probably makes the lock contention way worse, too,\n> because accessing existing objects just has one big coarse lock).\n> \n> So definitely do something like:\n> \n>   for threads in 1 2 3 4 5 12 56; do\n> \trm -rf repo.git\n> \tgit init --bare repo.git\n> \tGIT_FORCE_THREADS=$threads \\\n> \t  git -C repo.git index-pack -v --stdin </path/to/pack\n>   done\n> \n> to test.\n\nThis is roughly what p5302 is going, though it does not go as high as\n56 (though it seems like there is probably not much point in doing so).\n\nHowever, I did notice this slight bug in it. After this fix, here are my\nnumbers from indexing git.git:\n\n  Test                                           HEAD             \n  ----------------------------------------------------------------\n  5302.2: index-pack 0 threads                   22.72(22.55+0.16)\n  5302.3: index-pack 1 thread                    23.26(23.02+0.24)\n  5302.4: index-pack 2 threads                   13.19(24.06+0.23)\n  5302.5: index-pack 4 threads                   7.96(24.65+0.25) \n  5302.6: index-pack 8 threads                   7.94(45.06+0.38) \n  5302.7: index-pack default number of threads   9.37(23.82+0.18) \n\nSo it looks like \"4\" is slightly better than the default of \"3\" for me.\n\nI'm running it on linux.git now, but it will take quite a while to come\nup with a result.\n\n-- >8 --\nSubject: [PATCH] p5302: create the repo in each index-pack test\n\nThe p5302 script runs \"index-pack --stdin\" in each timing test. It does\ntwo things to try to get good timings:\n\n  1. we do the repo creation in a separate (non-timed) setup test, so\n     that our timing is purely the index-pack run\n\n  2. we use a separate repo for each test; this is important because the\n     presence of existing objects in the repo influences the result\n     (because we'll end up doing collision checks against them)\n\nBut this forgets one thing: we generally run each timed test multiple\ntimes to reduce the impact of noise. Which means that repeats of each\ntest after the first will be subject to the collision slowdown from\npoint 2, and we'll generally just end up taking the first time anyway.\n\nInstead, let's create the repo in the test (effectively undoing point\n1). That does add a constant amount of extra work to each iteration, but\nit's quite small compared to the actual effects we're interested in\nmeasuring.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe very first 0-thread one will run faster because it has less to \"rm\n-rf\", but I think we can ignore that.\n\n t/perf/p5302-pack-index.sh | 31 ++++++++++++++++++-------------\n 1 file changed, 18 insertions(+), 13 deletions(-)\n\ndiff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh\nindex 99bdb16c85..a9b3e112d9 100755\n--- a/t/perf/p5302-pack-index.sh\n+++ b/t/perf/p5302-pack-index.sh\n@@ -13,35 +13,40 @@ test_expect_success 'repack' '\n \texport PACK\n '\n \n-test_expect_success 'create target repositories' '\n-\tfor repo in t1 t2 t3 t4 t5 t6\n-\tdo\n-\t\tgit init --bare $repo\n-\tdone\n-'\n-\n test_perf 'index-pack 0 threads' '\n-\tGIT_DIR=t1 git index-pack --threads=1 --stdin < $PACK\n+\trm -rf repo.git &&\n+\tgit init --bare repo.git &&\n+\tGIT_DIR=repo.git git index-pack --threads=1 --stdin < $PACK\n '\n \n test_perf 'index-pack 1 thread ' '\n-\tGIT_DIR=t2 GIT_FORCE_THREADS=1 git index-pack --threads=1 --stdin < $PACK\n+\trm -rf repo.git &&\n+\tgit init --bare repo.git &&\n+\tGIT_DIR=repo.git GIT_FORCE_THREADS=1 git index-pack --threads=1 --stdin < $PACK\n '\n \n test_perf 'index-pack 2 threads' '\n-\tGIT_DIR=t3 git index-pack --threads=2 --stdin < $PACK\n+\trm -rf repo.git &&\n+\tgit init --bare repo.git &&\n+\tGIT_DIR=repo.git git index-pack --threads=2 --stdin < $PACK\n '\n \n test_perf 'index-pack 4 threads' '\n-\tGIT_DIR=t4 git index-pack --threads=4 --stdin < $PACK\n+\trm -rf repo.git &&\n+\tgit init --bare repo.git &&\n+\tGIT_DIR=repo.git git index-pack --threads=4 --stdin < $PACK\n '\n \n test_perf 'index-pack 8 threads' '\n-\tGIT_DIR=t5 git index-pack --threads=8 --stdin < $PACK\n+\trm -rf repo.git &&\n+\tgit init --bare repo.git &&\n+\tGIT_DIR=repo.git git index-pack --threads=8 --stdin < $PACK\n '\n \n test_perf 'index-pack default number of threads' '\n-\tGIT_DIR=t6 git index-pack --stdin < $PACK\n+\trm -rf repo.git &&\n+\tgit init --bare repo.git &&\n+\tGIT_DIR=repo.git git index-pack --stdin < $PACK\n '\n \n test_done\n-- \n2.21.0.1182.g3590c06d32\n\n"},{"id":"374276","messageId":"19221376.OlD5LWjr85@mfick-lnx","threadId":"50956","inReplyTo":"20190422205653.GA30286@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2019-04-22T22:32:16Z","receivedAt":"2019-04-22T22:32:20Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Monday, April 22, 2019 4:56:54 PM MDT Jeff King wrote:\n> On Mon, Apr 22, 2019 at 02:21:40PM -0600, Martin Fick wrote:\n> > > Try this (with a recent version of git; your v1.8.2.1 won't have\n> > > \n> > > --batch-all-objects):\n> > >   # count the on-disk size of all objects\n> > >   git cat-file --batch-all-objects --batch-check='%(objectsize)\n> > > \n> > > %(objectsize:disk)' | perl -alne '\n> > > \n> > >     $repo += $F[0];\n> > >     $disk += $F[1];\n> > >     END { print \"$repo / $disk = \", $repo/$disk }\n> > >   \n> > >   '\n> > \n> > This has been running for a few hours now, I will update you with results\n> > when its done.\n> \n> Hours? I think something might be wrong. It takes 20s to run on\n> linux.git.\n\nOK, yes I was running this on a \"bad\" copy of the repo, see below because I \nthink it might be of some interest also...\n\nOn the better copy, the git part of the command took 3m48.025s (still not 20s, \nbut not hours either), using git v2.18. The results were:\n\n2085532480789 / 7909358121 = 263.679106304687\n\nThis seems to confirm the sizing numbers.\n\n\nAs for the \"bad repo\", let me describe:\n\n1) It is on a different machine (only 32 processors, and some serious \nbackground load)\n\n2) That copy of the repo is from a mirror clone of our source repo without \nrunning repacking (-f) on it. Without repacking -f, the clone is actually 16G, \nwhich I believe is using the deltas from the source repo which would have been \ncreated by people pushing new commits over and over to the source repo and \nthose new objects just being added to the common pack file during repacking \nwithout creating a ton of new deltas. It would appear that the deltas created \nfrom regular repacking without using the -f may be really destroying some of \nthe performance of our source repo even worse then I imagined. The rest of my \ntesting has been done on a repo repacked with -f to eliminate the variability \nimposed from the source repo because I figured it would adversely impact \nthings, but I did not imagine it being that bad because even the clone time of \nthe bad repo is not that bad, so I wonder why the git cat-file is way worse?\n\n\n> The key is really having objects where the size of change versus the\n> file size is small (so the marginal cost of each revision is small), and\n> then having lots of changes (to give you a big multiplier).\n\nRight.\n\n> > > So yes, if you really do have to hash 2TB of data, that's going to take\n> > > a while.\n> > \n> > I was hoping I was wrong. Unfortunately I sense that this is not likely\n> > something we can improve with a better algorithm. It seems like the best\n> > way to handle this long term is likely to use BUP's rolling hash\n> > splitting, it would make this way better (assuming it made objects small\n> > enough). I think it is interesting that this approach might end up being\n> > effective for more than just large binary file repos. If I could get this\n> > repo into bup somehow, it could potentially show us if this would\n> > drastically reduce the index-pack time.\n> \n> It fundamentally can't help without changing Git's object model, since\n> you need to have complete hashes of all of those files. If you're\n> proposing to do rolling hashes to split blobs into multiple objects that\n> would definitely work. But it wouldn't be compatible with Git anymore.\n\nRight. I really just meant to point out how many people may want the BUP style \nrolling hash split blobs in git even without having gigantic blobs since it \nmight really help with smaller blobs and long histories with small deltas. \nMost of the git performance issues tend to focus on large repos with large \nblobs, that is what BUP was made for, but it could really help normal git \nusers, possibly even the kernel as we start to develop some seriously longer \nhistories. I think most assumptions have been that this is not likely to be a \nproblem any time soon.\n\n> Are you running it in the same repo, or in another newly-created repo?\n\nYes.\n\n> Or alternatively, in a new repo but repeatedly running index-pack? After\n> the first run, that repo will have all of the objects. And so for each\n> object it sees, index-pack will say \"woah, we already had that one;\n> let's double check that they're byte for byte identical\" which carries\n> extra overhead (and probably makes the lock contention way worse, too,\n> because accessing existing objects just has one big coarse lock).\n> \n> So definitely do something like:\n> \n>   for threads in 1 2 3 4 5 12 56; do\n> \trm -rf repo.git\n> \tgit init --bare repo.git\n> \tGIT_FORCE_THREADS=$threads \\\n> \t  git -C repo.git index-pack -v --stdin </path/to/pack\n>   done\n> \n> to test.\n\nMakes sense, testing now...\n\n> > Perhaps the locking can be improved here to make threading more effective?\n> \n> Probably, but easier said than done, of course.\n> \n> > > We also hash non-deltas while we're receiving them over the network.\n> > > That's accounted for in the \"receiving pack\" part of the progress meter.\n> > > If the time looks to be going to \"resolving deltas\", then that should\n> > > all be threaded.\n> > \n> > Would it make sense to make the receiving pack time also threaded because\n> > I\n> > believe that time is still longer than the I/O time (2 or 3 times)?\n> \n> It's a lot harder to thread since we're eating the incoming bytes. And\n> unless you're just coming from a local disk copy, the network is\n> generally the bottleneck (and if you are coming from a local disk copy,\n> then consider doing a local clone which will avoid all of this hashing\n> in the first place).\n\nIs that really a fair assumption in today's intranets? Many corporate LANs \nhave higher bandwidth than normal disks, don't they?\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n\n"},{"id":"374280","messageId":"xmqqef5t7cil.fsf@gitster-ct.c.googlers.com","threadId":"50956","inReplyTo":"20190422211952.GA4728@sigill.intra.peff.net","subject":"Re: [PATCH] p5302: create the repo in each index-pack test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-23T01:09:54Z","receivedAt":"2019-04-23T01:10:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Subject: [PATCH] p5302: create the repo in each index-pack test\n>\n> The p5302 script runs \"index-pack --stdin\" in each timing test. It does\n> two things to try to get good timings:\n>\n>   1. we do the repo creation in a separate (non-timed) setup test, so\n>      that our timing is purely the index-pack run\n>\n>   2. we use a separate repo for each test; this is important because the\n>      presence of existing objects in the repo influences the result\n>      (because we'll end up doing collision checks against them)\n>\n> But this forgets one thing: we generally run each timed test multiple\n> times to reduce the impact of noise. Which means that repeats of each\n> test after the first will be subject to the collision slowdown from\n> point 2, and we'll generally just end up taking the first time anyway.\n\nThe above is very cleanly written to convince anybody that what the\ncurrent test does contradicts with wish #2 above, and that the two\nwishes #1 and #2 are probably mutually incompatible.\n\nBut isn't the collision check a part of the real-life workload that\nGit users are made waiting for and care about the performance of?\nOr are we purely interested in the cost of resolving delta,\ncomputing the object name, and writing the result out to the disk in\nthis test and the \"overall experience\" benchmark is left elsewhere?\n\nThe reason why I got confused is because the test_description of the\nscript leaves \"the actual effects we're interested in measuring\"\nunsaid, I think.  The log message of b8a2486f (\"index-pack: support\nmultithreaded delta resolving\", 2012-05-06) that created this test\ndoes not help that much, either.\n\nIn any case, the above \"this forgets one thing\" makes it clear that\nwe at this point in time declare what we are interested in very\nclearly, and I agree that the solution described in the paragraph\nbelow clearly matches the goal.  Looks good.\n\n> Instead, let's create the repo in the test (effectively undoing point\n> 1). That does add a constant amount of extra work to each iteration, but\n> it's quite small compared to the actual effects we're interested in\n> measuring.\n\n\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> The very first 0-thread one will run faster because it has less to \"rm\n> -rf\", but I think we can ignore that.\n\nOK.\n\n> -\tGIT_DIR=t1 git index-pack --threads=1 --stdin < $PACK\n> +\trm -rf repo.git &&\n> +\tgit init --bare repo.git &&\n> +\tGIT_DIR=repo.git git index-pack --threads=1 --stdin < $PACK\n\nThis is obviously inherited from the original, but do we get scolded\nby some versions of bash for this line, without quoting the source path\nof the redirection, i.e.\n\n\t... --stdin <\"$PACK\"\n\n"},{"id":"374284","messageId":"20190423015538.GA16369@sigill.intra.peff.net","threadId":"50956","inReplyTo":"19221376.OlD5LWjr85@mfick-lnx","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-23T01:55:38Z","receivedAt":"2019-04-23T01:55:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 22, 2019 at 04:32:16PM -0600, Martin Fick wrote:\n\n> > Hours? I think something might be wrong. It takes 20s to run on\n> > linux.git.\n> \n> OK, yes I was running this on a \"bad\" copy of the repo, see below because I \n> think it might be of some interest also...\n> \n> On the better copy, the git part of the command took 3m48.025s (still not 20s, \n> but not hours either), using git v2.18. The results were:\n\nThat is slower than I'd expect. Two things that could speed it up:\n\n  - use --buffer, which drops write() overhead\n\n  - use a more recent version of Git which has 0750bb5b51 (cat-file:\n    support \"unordered\" output for --batch-all-objects, 2018-08-10).\n    This is order gets better cache locality (both disk cache if you\n    can't fit the whole thing in RAM, but also the delta cache).\n\nThose drop my 20s run on linux.git to 13s, but they might have more\nimpact for you.\n\n> 2) That copy of the repo is from a mirror clone of our source repo without \n> running repacking (-f) on it. Without repacking -f, the clone is actually 16G, \n> which I believe is using the deltas from the source repo which would have been \n> created by people pushing new commits over and over to the source repo and \n> those new objects just being added to the common pack file during repacking \n> without creating a ton of new deltas. It would appear that the deltas created \n> from regular repacking without using the -f may be really destroying some of \n> the performance of our source repo even worse then I imagined. The rest of my \n> testing has been done on a repo repacked with -f to eliminate the variability \n> imposed from the source repo because I figured it would adversely impact \n> things, but I did not imagine it being that bad because even the clone time of \n> the bad repo is not that bad, so I wonder why the git cat-file is way worse?\n\nYes, I do think it's worth doing a \"repack -f\" every once in a while\n(and probably worth --window=250 since you're already splurging on CPU).\nIt does seem to produce better results than taking the accumulated\nresults of the individual pushes. I don't think this is an area we've\nstudied all that well, so there may be some ways to make it better (but\nwhat's there has been \"good enough\" that nobody has sunk a lot of time\ninto it).\n\n> > So definitely do something like:\n> > \n> >   for threads in 1 2 3 4 5 12 56; do\n> > \trm -rf repo.git\n> > \tgit init --bare repo.git\n> > \tGIT_FORCE_THREADS=$threads \\\n> > \t  git -C repo.git index-pack -v --stdin </path/to/pack\n> >   done\n> > \n> > to test.\n> \n> Makes sense, testing now...\n\nHere are my p5302 numbers on linux.git, by the way.\n\n  Test                                           jk/p5302-repeat-fix\n  ------------------------------------------------------------------\n  5302.2: index-pack 0 threads                   307.04(303.74+3.30)\n  5302.3: index-pack 1 thread                    309.74(306.13+3.56)\n  5302.4: index-pack 2 threads                   177.89(313.73+3.60)\n  5302.5: index-pack 4 threads                   117.14(344.07+4.29)\n  5302.6: index-pack 8 threads                   112.40(607.12+5.80)\n  5302.7: index-pack default number of threads   135.00(322.03+3.74)\n\nwhich still imply that \"4\" is a win over \"3\" (\"8\" is slightly better\nstill in wall-clock time, but the total CPU rises dramatically; that's\nprobably because this is a quad-core with hyperthreading, so by that\npoint we're just throttling down the CPUs).\n\n> > It's a lot harder to thread since we're eating the incoming bytes. And\n> > unless you're just coming from a local disk copy, the network is\n> > generally the bottleneck (and if you are coming from a local disk copy,\n> > then consider doing a local clone which will avoid all of this hashing\n> > in the first place).\n> \n> Is that really a fair assumption in today's intranets? Many corporate LANs \n> have higher bandwidth than normal disks, don't they?\n\nSingle-threaded SHA-1 is 500-1000 megabytes/sec. That's 4-8\ngigabits/sec. And keep in mind we're just handling the base objects, so\nwe're only computing the SHA-1 on some of the bytes.\n\nOf course there's another SHA-1 computation going over the whole\npackfile at that point, so _that_ is probably a bigger bottleneck if you\nreally are on a 10 gigabit network.\n\nBut if somebody wants to spend the time to look at parallelizing it, I\nwouldn't say no. :)\n\n-Peff\n"},{"id":"374285","messageId":"20190423020749.GB16369@sigill.intra.peff.net","threadId":"50956","inReplyTo":"xmqqef5t7cil.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] p5302: create the repo in each index-pack test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-23T02:07:50Z","receivedAt":"2019-04-23T02:07:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 23, 2019 at 10:09:54AM +0900, Junio C Hamano wrote:\n\n> The above is very cleanly written to convince anybody that what the\n> current test does contradicts with wish #2 above, and that the two\n> wishes #1 and #2 are probably mutually incompatible.\n> \n> But isn't the collision check a part of the real-life workload that\n> Git users are made waiting for and care about the performance of?\n> Or are we purely interested in the cost of resolving delta,\n> computing the object name, and writing the result out to the disk in\n> this test and the \"overall experience\" benchmark is left elsewhere?\n\nI think we _are_ just interested in the resolving delta cost (after all,\nwe're testing it with various thread levels). What's more, the old code\nwould run the test $GIT_PERF_COUNT times, once without any objects, and\nthen the other N-1 times with objects. And then take the smallest time,\nwhich would generally be the one-off!  So we're really just measuring\nthat case more consistently now.\n\nBut even if you left all of that aside, I think the case without objects\nis actually the realistic one. It represents the equivalent a full\nclone, where we would not have any objects already.\n\nThe case where we are fetching into a repository with objects already is\nalso potentially of interest, but this test wouldn't show that very\nwell. Because there the main added cost is looking up each object and\nsaying \"ah, we do not have it; no need to do a collision check\", because\nwe'd generally not expect the other side to be sending us duplicates.\n\nBut because this test would be repeating itself on the same pack each\ntime, we'd be seeing a collision on _every_ object. And the added time\nwould be dominated by us saying \"oops, a collision; let's take the slow\npath and reconstruct that object from disk so we can compare its bytes\".\n\n> The reason why I got confused is because the test_description of the\n> script leaves \"the actual effects we're interested in measuring\"\n> unsaid, I think.  The log message of b8a2486f (\"index-pack: support\n> multithreaded delta resolving\", 2012-05-06) that created this test\n> does not help that much, either.\n> \n> In any case, the above \"this forgets one thing\" makes it clear that\n> we at this point in time declare what we are interested in very\n> clearly, and I agree that the solution described in the paragraph\n> below clearly matches the goal.  Looks good.\n\nSo I think you convinced yourself even before my email that this was the\nright path, but let me know if you think it's worth trying to revise the\ncommit message to include some of the above reasoning.\n\n> > -\tGIT_DIR=t1 git index-pack --threads=1 --stdin < $PACK\n> > +\trm -rf repo.git &&\n> > +\tgit init --bare repo.git &&\n> > +\tGIT_DIR=repo.git git index-pack --threads=1 --stdin < $PACK\n> \n> This is obviously inherited from the original, but do we get scolded\n> by some versions of bash for this line, without quoting the source path\n> of the redirection, i.e.\n> \n> \t... --stdin <\"$PACK\"\n\nIn general, yes, but I think we are OK in this instance because we\ngenerated $PACK ourselves in the setup step, and we know that it is just\na relative .git/objects/pack/xyz.pack with no spaces. I almost touched\nit just to get rid of the style-violating space after the \"<\" though. ;)\n\n-Peff\n"},{"id":"374288","messageId":"xmqqv9z55udl.fsf@gitster-ct.c.googlers.com","threadId":"50956","inReplyTo":"20190423020749.GB16369@sigill.intra.peff.net","subject":"Re: [PATCH] p5302: create the repo in each index-pack test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-23T02:27:02Z","receivedAt":"2019-04-23T02:27:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> This is obviously inherited from the original, but do we get scolded\n>> by some versions of bash for this line, without quoting the source path\n>> of the redirection, i.e.\n>> \n>> \t... --stdin <\"$PACK\"\n>\n> In general, yes, but I think we are OK in this instance because we\n> generated $PACK ourselves in the setup step, and we know that it is just\n> a relative .git/objects/pack/xyz.pack with no spaces.\n\nI know we are OK, but the issue with some versions of bash AFAIU is\nthat bash is not OK regardless of the contents of $variable that is\nnot quoted and used as the target or the source of a redirection,\nissuing an unnecessary warning.\n\n"},{"id":"374289","messageId":"20190423023651.GD16369@sigill.intra.peff.net","threadId":"50956","inReplyTo":"xmqqv9z55udl.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] p5302: create the repo in each index-pack test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-23T02:36:51Z","receivedAt":"2019-04-23T02:36:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 23, 2019 at 11:27:02AM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> >> This is obviously inherited from the original, but do we get scolded\n> >> by some versions of bash for this line, without quoting the source path\n> >> of the redirection, i.e.\n> >> \n> >> \t... --stdin <\"$PACK\"\n> >\n> > In general, yes, but I think we are OK in this instance because we\n> > generated $PACK ourselves in the setup step, and we know that it is just\n> > a relative .git/objects/pack/xyz.pack with no spaces.\n> \n> I know we are OK, but the issue with some versions of bash AFAIU is\n> that bash is not OK regardless of the contents of $variable that is\n> not quoted and used as the target or the source of a redirection,\n> issuing an unnecessary warning.\n\nIs it? I thought the issue was specifically when there were spaces. I\nget:\n\n  $ bash\n  $ file=ok\n  $ echo foo >$file\n  $ file='not ok'\n  $ echo foo >$file\n  bash: $file: ambiguous redirect\n\nAnd that is AFAIK what the recent 7951a016a5 (t4038-diff-combined: quote\npaths with whitespace, 2019-03-17) was about (because our trash\ndirectory always has a space in it).\n\nDid I miss a report where it happens on some versions even without\nspaces? If so, we have quite a number of things to fix judging from the\noutput of:\n\n  git grep '>\\$'\n\n-Peff\n"},{"id":"374290","messageId":"xmqqr29t5tr2.fsf@gitster-ct.c.googlers.com","threadId":"50956","inReplyTo":"20190423023651.GD16369@sigill.intra.peff.net","subject":"Re: [PATCH] p5302: create the repo in each index-pack test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-23T02:40:33Z","receivedAt":"2019-04-23T02:40:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Is it? I thought the issue was specifically when there were spaces. I\n> get:\n>\n>   $ bash\n>   $ file=ok\n>   $ echo foo >$file\n>   $ file='not ok'\n>   $ echo foo >$file\n>   bash: $file: ambiguous redirect\n\nOK, so I misremembered.  Then we are good.  Thanks.\n"},{"id":"374297","messageId":"20190423042109.GA19183@sigill.intra.peff.net","threadId":"50956","inReplyTo":"20190423015538.GA16369@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-23T04:21:09Z","receivedAt":"2019-04-23T04:21:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 22, 2019 at 09:55:38PM -0400, Jeff King wrote:\n\n> Here are my p5302 numbers on linux.git, by the way.\n> \n>   Test                                           jk/p5302-repeat-fix\n>   ------------------------------------------------------------------\n>   5302.2: index-pack 0 threads                   307.04(303.74+3.30)\n>   5302.3: index-pack 1 thread                    309.74(306.13+3.56)\n>   5302.4: index-pack 2 threads                   177.89(313.73+3.60)\n>   5302.5: index-pack 4 threads                   117.14(344.07+4.29)\n>   5302.6: index-pack 8 threads                   112.40(607.12+5.80)\n>   5302.7: index-pack default number of threads   135.00(322.03+3.74)\n> \n> which still imply that \"4\" is a win over \"3\" (\"8\" is slightly better\n> still in wall-clock time, but the total CPU rises dramatically; that's\n> probably because this is a quad-core with hyperthreading, so by that\n> point we're just throttling down the CPUs).\n\nAnd here's a similar test run on a 20-core Xeon w/ hyperthreading (I\ntweaked the test to keep going after eight threads):\n\nTest                            HEAD                \n----------------------------------------------------\n5302.2: index-pack 1 threads    376.88(364.50+11.52)\n5302.3: index-pack 2 threads    228.13(371.21+17.86)\n5302.4: index-pack 4 threads    151.41(387.06+21.12)\n5302.5: index-pack 8 threads    113.68(413.40+25.80)\n5302.6: index-pack 16 threads   100.60(511.85+37.53)\n5302.7: index-pack 32 threads   94.43(623.82+45.70) \n5302.8: index-pack 40 threads   93.64(702.88+47.61) \n\nI don't think any of this is _particularly_ relevant to your case, but\nit really seems to me that the default of capping at 3 threads is too\nlow.\n\n-Peff\n"},{"id":"374303","messageId":"8736m9td2d.fsf@evledraar.gmail.com","threadId":"50956","inReplyTo":"20190422184329.GA20304@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-23T07:07:06Z","receivedAt":"2019-04-23T07:07:11Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Apr 22 2019, Jeff King wrote:\n\n> On Mon, Apr 22, 2019 at 08:01:15PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > Your patch is optionally removing the \"woah, we got an object with a\n>> > duplicate sha1, let's check that the bytes are the same in both copies\"\n>> > check. But Martin's problem is a clone, so we wouldn't have any existing\n>> > objects to duplicate in the first place.\n>>\n>> Right, but we do anyway, as reported by Geert at @amazon.com preceding\n>> that patch of mine. But it is 99.99% irrelevant to *performance* in this\n>> case after the loose object cache you added (but before that could make\n>> all the difference depending on the FS).\n>\n> I scratched my head at this a bit. If we don't have any other objects,\n> then what are we comparing against? But I think you mean that we have\n> the overhead of doing the object lookups to find that out. Yes, that can\n> add up if your filesystem has high latency, but I think in this case it\n> is a drop in the bucket compared to dealing with the actual object data.\n\nThere was no \"we have no objects\" clause, so this bit is what dominated\nclone time before the loose object cache...\n\n>> I just mentioned it to plant a flag on another bit of the code where\n>> index-pack in general has certain paranoias/validation the user might be\n>> willing to optionally drop just at \"clone\" time.\n>\n> Yeah, I agree it may be worth pursuing independently. I just don't think\n> it will help Martin's case in any noticeable way.\n\n...indeed, as noted just mentioning this in the context of things in\nindex-pack that *in general* might benefit from some \"we had no objects\nbefore\" special-case.\n\n>> > Right, that would work. I will note one thing, though: the total time to\n>> > do a 1-depth clone followed by an unshallow is probably much higher than\n>> > doing the whole clone as one unit, for two reasons:\n>>\n>> Indeed. The hypothesis is that the user doesn't really care about the\n>> clone-time, but the clone-to-repo-mostly-usable time.\n>\n> There was a little bit of self-interest in there for me, too, as a\n> server operator. While it does add to the end-to-end time, most of the\n> resource use for the shallow fetch gets put on the server. IOW, I don't\n> think we'd be happy to see clients doing this depth-1-and-then-unshallow\n> strategy for every clone.\n\nJust change from per-seat pricing to charging a premium for CPU &\nI/O. Now your problem is a solution :)\n\nMore seriously, yeah I think we definitely need to be careful about\nchanges to git that'll eat someone \"free\" server time to save the client\ntime/work.\n\nAt the same time we have dedicated internal operators who wouldn't mind\nspending that CPU. So hopefully we can in general find some reasonable\nmiddle-ground.\n\n>> > So in general, I think you'd need some cooperation from the server side\n>> > to ask it to generate and send the .idx that matches the .pack it is\n>> > sending you. Or even if not the .idx format itself, some stable list of\n>> > sha1s that you could use to reproduce it without hashing each\n>> > uncompressed byte yourself.\n>>\n>> Yeah, depending on how jt/fetch-cdn-offload is designed (see my\n>> https://public-inbox.org/git/87k1hv6eel.fsf@evledraar.gmail.com/) it\n>> could be (ab)used to do this. I.e. you'd keep a \"base\" *.{pack,idx}\n>> around for such a purpose.\n>>\n>> So in such a case you'd serve up that recent-enough *.{pack,idx} for the\n>> client to \"wget\", and the client would then trust it (or not) and do the\n>> equivalent of a \"fetch\" from that point to be 100% up-to-date.\n>\n> I think it's sort of orthogonal. Either way you have to teach the client\n> how to get a .pack/.idx combo. Whether it learns to receive it inline\n> from the first fetch, or whether it is taught to expect it from the\n> out-of-band fetch, most of the challenge is the same.\n\nI think it is, but maybe we're talking about different things.\n\nI suspect a few of us have experimented with similar rsync-and-pull\nhacks as a replacement for \"clone\". It's much faster (often 50-90%\nfaster).\n\nI.e. just an rsync of a recent-enough .git dir (or .git/objects),\nfollowed by a 'reset --hard' to get the worktree and then a 'git pull'.\n\n>> > This could even be stuffed into the pack format and stripped out by\n>> > the receiving index-pack (i.e., each entry is prefixed with \"and by\n>> > the way, here is my sha1...\").\n>>\n>> That would be really interesting. I.e. just having room for that (or\n>> anything else) in the pack format.\n>>\n>> I wonder if it could be added to the delta-chain in the current format\n>> as a nasty hack :)\n>\n> There's definitely not \"room\" in any sense of the word in the pack\n> format. :) However, as long as all parties agreed, we can stick whatever\n> we want into the on-the-wire format. So I was imagining something more\n> like:\n>\n>   1. pack-objects learns a --report-object-id option that sticks some\n>      additional bytes before each object (in its simplest form,\n>      $obj_hash bytes of id)\n>\n>   2. likewise, index-pack learns a --parse-object-id option to receive\n>      it and skip hashing the object bytes\n>\n>   3. we get a new protocol capability, \"send-object-ids\". If the server\n>      advertises and the client requests it, then both sides turn on the\n>      appropriate option\n>\n> You could even imagine generalizing it to \"--report-object-metadata\",\n> and including 0 or more metadata packets before each object. With object\n> id being one, but possibly other computable bits like \"generation\n> number\" after that. I'm not convinced other metadata is worth the\n> space/time tradeoff, though. After all, this is stuff that the client\n> _could_ generate and cache themselves, so you're trading off bandwidth\n> to save the client from doing the computation.\n>\n> Anyway, food for thought. :)\n"},{"id":"374312","messageId":"CACsJy8B7tjjpUZK+zH4rvOSk=uTLOHCOy6hk4FkkHXqCzNZU9g@mail.gmail.com","threadId":"50956","inReplyTo":"20190423042109.GA19183@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-04-23T10:08:40Z","receivedAt":"2019-04-23T10:09:11Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Apr 23, 2019 at 11:45 AM Jeff King <peff@peff.net> wrote:\n>\n> On Mon, Apr 22, 2019 at 09:55:38PM -0400, Jeff King wrote:\n>\n> > Here are my p5302 numbers on linux.git, by the way.\n> >\n> >   Test                                           jk/p5302-repeat-fix\n> >   ------------------------------------------------------------------\n> >   5302.2: index-pack 0 threads                   307.04(303.74+3.30)\n> >   5302.3: index-pack 1 thread                    309.74(306.13+3.56)\n> >   5302.4: index-pack 2 threads                   177.89(313.73+3.60)\n> >   5302.5: index-pack 4 threads                   117.14(344.07+4.29)\n> >   5302.6: index-pack 8 threads                   112.40(607.12+5.80)\n> >   5302.7: index-pack default number of threads   135.00(322.03+3.74)\n> >\n> > which still imply that \"4\" is a win over \"3\" (\"8\" is slightly better\n> > still in wall-clock time, but the total CPU rises dramatically; that's\n> > probably because this is a quad-core with hyperthreading, so by that\n> > point we're just throttling down the CPUs).\n>\n> And here's a similar test run on a 20-core Xeon w/ hyperthreading (I\n> tweaked the test to keep going after eight threads):\n>\n> Test                            HEAD\n> ----------------------------------------------------\n> 5302.2: index-pack 1 threads    376.88(364.50+11.52)\n> 5302.3: index-pack 2 threads    228.13(371.21+17.86)\n> 5302.4: index-pack 4 threads    151.41(387.06+21.12)\n> 5302.5: index-pack 8 threads    113.68(413.40+25.80)\n> 5302.6: index-pack 16 threads   100.60(511.85+37.53)\n> 5302.7: index-pack 32 threads   94.43(623.82+45.70)\n> 5302.8: index-pack 40 threads   93.64(702.88+47.61)\n>\n> I don't think any of this is _particularly_ relevant to your case, but\n> it really seems to me that the default of capping at 3 threads is too\n> low.\n\nLooking back at the multithread commit, I think the trend was the same\nand I capped it because the gain was not proportional to the number of\ncores we threw at index-pack anymore. I would not be opposed to\nraising the cap though (or maybe just remove it)\n-- \nDuy\n"},{"id":"374327","messageId":"3329645.KIYB9vJKXd@mfick-lnx","threadId":"50956","inReplyTo":"CACsJy8B7tjjpUZK+zH4rvOSk=uTLOHCOy6hk4FkkHXqCzNZU9g@mail.gmail.com","subject":"Re: Resolving deltas dominates clone time","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2019-04-23T20:09:31Z","receivedAt":"2019-04-23T20:09:36Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Tuesday, April 23, 2019 5:08:40 PM MDT Duy Nguyen wrote:\n> On Tue, Apr 23, 2019 at 11:45 AM Jeff King <peff@peff.net> wrote:\n> > On Mon, Apr 22, 2019 at 09:55:38PM -0400, Jeff King wrote:\n> > > Here are my p5302 numbers on linux.git, by the way.\n> > > \n> > >   Test                                           jk/p5302-repeat-fix\n> > >   ------------------------------------------------------------------\n> > >   5302.2: index-pack 0 threads                   307.04(303.74+3.30)\n> > >   5302.3: index-pack 1 thread                    309.74(306.13+3.56)\n> > >   5302.4: index-pack 2 threads                   177.89(313.73+3.60)\n> > >   5302.5: index-pack 4 threads                   117.14(344.07+4.29)\n> > >   5302.6: index-pack 8 threads                   112.40(607.12+5.80)\n> > >   5302.7: index-pack default number of threads   135.00(322.03+3.74)\n> > > \n> > > which still imply that \"4\" is a win over \"3\" (\"8\" is slightly better\n> > > still in wall-clock time, but the total CPU rises dramatically; that's\n> > > probably because this is a quad-core with hyperthreading, so by that\n> > > point we're just throttling down the CPUs).\n> > \n> > And here's a similar test run on a 20-core Xeon w/ hyperthreading (I\n> > tweaked the test to keep going after eight threads):\n> > \n> > Test                            HEAD\n> > ----------------------------------------------------\n> > 5302.2: index-pack 1 threads    376.88(364.50+11.52)\n> > 5302.3: index-pack 2 threads    228.13(371.21+17.86)\n> > 5302.4: index-pack 4 threads    151.41(387.06+21.12)\n> > 5302.5: index-pack 8 threads    113.68(413.40+25.80)\n> > 5302.6: index-pack 16 threads   100.60(511.85+37.53)\n> > 5302.7: index-pack 32 threads   94.43(623.82+45.70)\n> > 5302.8: index-pack 40 threads   93.64(702.88+47.61)\n> > \n> > I don't think any of this is _particularly_ relevant to your case, but\n> > it really seems to me that the default of capping at 3 threads is too\n> > low.\n\nHere are my index-pack results (I only ran them once since they take a while) \nusing vgit 1.8.3.2:\n\nThreads  real       user        sys\n1     108m46.151s 106m14.420s  1m57.192s\n2     58m14.274s  106m23.158s  5m32.736s\n3     40m33.351s  106m42.281s  5m40.884s\n4     31m40.342s  107m20.278s  5m40.675s\n5     26m0.454s   106m54.370s  5m35.827s\n12    13m25.304s  107m57.271s  6m26.493s\n16    10m56.866s  107m46.107s  6m41.330s\n18    10m18.112s  109m50.893s  7m1.369s\n20    9m54.010s   113m51.028s  7m53.082s\n24    9m1.104s    115m8.245s   7m57.156s\n28    8m26.058s   116m46.311s  8m34.752s\n32    8m42.967s   140m33.280s  9m59.514s\n36    8m52.228s   151m28.939s  11m55.590s\n40    8m22.719s   153m4.496s   12m36.041s\n44    8m12.419s   166m41.594s  14m7.717s\n48    8m0.377s    172m3.597s   16m32.041s\n56    8m22.320s   188m31.426s  17m48.274s\n\n \n> Looking back at the multithread commit, I think the trend was the same\n> and I capped it because the gain was not proportional to the number of\n> cores we threw at index-pack anymore. I would not be opposed to\n> raising the cap though (or maybe just remove it)\n\nI think that if there were no default limit during a clone it could have \ndisastrous effects on people using the repo tool from the android project, or \nany other \"submodule like\" tool that might clone many projects in parallel. \nWith the repo tool, people often use a large -j number such as 24 which means \nthey end up cloning around 24 projects at a time, and they may do this for \naround 1000 projects. If git clone suddenly started as many threads as there \nare CPUs for each clone, this would likely paralyze the machine.\n\nI do suspect it would be nice to have a switch though that repo could use to \nadjust this intelligently, is there some way to adjust threads from a clone, I \ndon't see one? I tried using 'GIT_FORCE_THREADS=28 git clone ...' and it \ndidn't seem to make a difference?\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n\n"},{"id":"374704","messageId":"20190430175048.GB16729@sigill.intra.peff.net","threadId":"50956","inReplyTo":"CACsJy8B7tjjpUZK+zH4rvOSk=uTLOHCOy6hk4FkkHXqCzNZU9g@mail.gmail.com","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-30T17:50:48Z","receivedAt":"2019-04-30T17:50:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 23, 2019 at 05:08:40PM +0700, Duy Nguyen wrote:\n\n> On Tue, Apr 23, 2019 at 11:45 AM Jeff King <peff@peff.net> wrote:\n> >\n> > On Mon, Apr 22, 2019 at 09:55:38PM -0400, Jeff King wrote:\n> >\n> > > Here are my p5302 numbers on linux.git, by the way.\n> > >\n> > >   Test                                           jk/p5302-repeat-fix\n> > >   ------------------------------------------------------------------\n> > >   5302.2: index-pack 0 threads                   307.04(303.74+3.30)\n> > >   5302.3: index-pack 1 thread                    309.74(306.13+3.56)\n> > >   5302.4: index-pack 2 threads                   177.89(313.73+3.60)\n> > >   5302.5: index-pack 4 threads                   117.14(344.07+4.29)\n> > >   5302.6: index-pack 8 threads                   112.40(607.12+5.80)\n> > >   5302.7: index-pack default number of threads   135.00(322.03+3.74)\n> > >\n> > > which still imply that \"4\" is a win over \"3\" (\"8\" is slightly better\n> > > still in wall-clock time, but the total CPU rises dramatically; that's\n> > > probably because this is a quad-core with hyperthreading, so by that\n> > > point we're just throttling down the CPUs).\n> >\n> > And here's a similar test run on a 20-core Xeon w/ hyperthreading (I\n> > tweaked the test to keep going after eight threads):\n> >\n> > Test                            HEAD\n> > ----------------------------------------------------\n> > 5302.2: index-pack 1 threads    376.88(364.50+11.52)\n> > 5302.3: index-pack 2 threads    228.13(371.21+17.86)\n> > 5302.4: index-pack 4 threads    151.41(387.06+21.12)\n> > 5302.5: index-pack 8 threads    113.68(413.40+25.80)\n> > 5302.6: index-pack 16 threads   100.60(511.85+37.53)\n> > 5302.7: index-pack 32 threads   94.43(623.82+45.70)\n> > 5302.8: index-pack 40 threads   93.64(702.88+47.61)\n> >\n> > I don't think any of this is _particularly_ relevant to your case, but\n> > it really seems to me that the default of capping at 3 threads is too\n> > low.\n> \n> Looking back at the multithread commit, I think the trend was the same\n> and I capped it because the gain was not proportional to the number of\n> cores we threw at index-pack anymore. I would not be opposed to\n> raising the cap though (or maybe just remove it)\n\nI'm not sure what the right cap would be. I don't think it's static;\nwe'd want ~4 threads on the top case, and 10-20 on the bottom one.\n\nIt does seem like there's an inflection point in the graph at N/2\nthreads. But then maybe that's just because these are hyper-threaded\nmachines, so \"N/2\" is the actual number of physical cores, and the\ninflated CPU times above that are just because we can't turbo-boost\nthen, so we're actually clocking slower. Multi-threaded profiling and\nmeasurement is such a mess. :)\n\nSo I'd say the right answer is probably either online_cpus() or half\nthat. The latter would be more appropriate for the machines I have, but\nI'd worry that it would leave performance on the table for non-intel\nmachines.\n\n-Peff\n"},{"id":"374705","messageId":"20190430180231.GC16729@sigill.intra.peff.net","threadId":"50956","inReplyTo":"3329645.KIYB9vJKXd@mfick-lnx","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-30T18:02:32Z","receivedAt":"2019-04-30T18:02:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 23, 2019 at 02:09:31PM -0600, Martin Fick wrote:\n\n> Here are my index-pack results (I only ran them once since they take a while) \n> using vgit 1.8.3.2:\n> \n> Threads  real       user        sys\n> 1     108m46.151s 106m14.420s  1m57.192s\n> 2     58m14.274s  106m23.158s  5m32.736s\n> 3     40m33.351s  106m42.281s  5m40.884s\n> 4     31m40.342s  107m20.278s  5m40.675s\n> 5     26m0.454s   106m54.370s  5m35.827s\n> 12    13m25.304s  107m57.271s  6m26.493s\n> 16    10m56.866s  107m46.107s  6m41.330s\n> 18    10m18.112s  109m50.893s  7m1.369s\n> 20    9m54.010s   113m51.028s  7m53.082s\n> 24    9m1.104s    115m8.245s   7m57.156s\n> 28    8m26.058s   116m46.311s  8m34.752s\n> 32    8m42.967s   140m33.280s  9m59.514s\n> 36    8m52.228s   151m28.939s  11m55.590s\n> 40    8m22.719s   153m4.496s   12m36.041s\n> 44    8m12.419s   166m41.594s  14m7.717s\n> 48    8m0.377s    172m3.597s   16m32.041s\n> 56    8m22.320s   188m31.426s  17m48.274s\n\nThanks for the data.\n\nThat seems to roughly match my results. Things get obviously better up\nto around close to half of the available processors, and then you get\nminimal returns for more CPU (some of yours actually get worse in the\nmiddle, but that may be due to noise; my timings are all best-of-3).\n\n> I think that if there were no default limit during a clone it could have \n> disastrous effects on people using the repo tool from the android project, or \n> any other \"submodule like\" tool that might clone many projects in parallel. \n> With the repo tool, people often use a large -j number such as 24 which means \n> they end up cloning around 24 projects at a time, and they may do this for \n> around 1000 projects. If git clone suddenly started as many threads as there \n> are CPUs for each clone, this would likely paralyze the machine.\n\nIMHO this is already a problem, because none of those 24 gits knows\nabout the others. So they're already using 24*3 cores, though of course\nat any given moment some of those 24 may be bottle-necked on the\nnetwork.\n\nI suspect that repo should be passing in `-c pack.threads=N`, where `N`\nis some formula based around how many cores we want to use, with some\nconstant fraction applied for how many we expect to be chugging on CPU\nat any given point.\n\nThe optimal behavior would probably come from index-pack dynamically\nassigning work based on system load, but that gets pretty messy. Ideally\nwe could just throw all of the load at the kernel's scheduler and let it\ndo the right thing, but:\n\n  - we clearly get some inefficiencies from being overly-parallelized,\n    so we don't want to go too far\n\n  - we have other resources we'd like to keep in use like the network\n    and disk. So probably the optimal case would be to have one (or a\n    few) index-packs fully utilizing the network, and then as they move\n    to the local-only CPU-heavy phase, start a few more on the network,\n    and so on.\n\n    There's no way to do that kind of slot-oriented gating now. It\n    actually wouldn't be too hard at the low-level of the code, but I'm\n    not sure what interface you'd use to communicate \"OK, now go\" to\n    each process.\n\n> I do suspect it would be nice to have a switch though that repo could use to \n> adjust this intelligently, is there some way to adjust threads from a clone, I \n> don't see one? I tried using 'GIT_FORCE_THREADS=28 git clone ...' and it \n> didn't seem to make a difference?\n\nI think I led you astray earlier by mentioning GIT_FORCE_THREADS. It's\nactually just a boolean for \"use threads even if we're only\nsingle-threaded\". What you actually want is probably:\n\n  git clone -c pack.threads=28 ...\n\n(though I didn't test it to be sure).\n\n-Peff\n"},{"id":"374713","messageId":"87sgtzqqhj.fsf@evledraar.gmail.com","threadId":"50956","inReplyTo":"20190430175048.GB16729@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-30T18:48:08Z","receivedAt":"2019-04-30T18:48:14Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Apr 30 2019, Jeff King wrote:\n\n> On Tue, Apr 23, 2019 at 05:08:40PM +0700, Duy Nguyen wrote:\n>\n>> On Tue, Apr 23, 2019 at 11:45 AM Jeff King <peff@peff.net> wrote:\n>> >\n>> > On Mon, Apr 22, 2019 at 09:55:38PM -0400, Jeff King wrote:\n>> >\n>> > > Here are my p5302 numbers on linux.git, by the way.\n>> > >\n>> > >   Test                                           jk/p5302-repeat-fix\n>> > >   ------------------------------------------------------------------\n>> > >   5302.2: index-pack 0 threads                   307.04(303.74+3.30)\n>> > >   5302.3: index-pack 1 thread                    309.74(306.13+3.56)\n>> > >   5302.4: index-pack 2 threads                   177.89(313.73+3.60)\n>> > >   5302.5: index-pack 4 threads                   117.14(344.07+4.29)\n>> > >   5302.6: index-pack 8 threads                   112.40(607.12+5.80)\n>> > >   5302.7: index-pack default number of threads   135.00(322.03+3.74)\n>> > >\n>> > > which still imply that \"4\" is a win over \"3\" (\"8\" is slightly better\n>> > > still in wall-clock time, but the total CPU rises dramatically; that's\n>> > > probably because this is a quad-core with hyperthreading, so by that\n>> > > point we're just throttling down the CPUs).\n>> >\n>> > And here's a similar test run on a 20-core Xeon w/ hyperthreading (I\n>> > tweaked the test to keep going after eight threads):\n>> >\n>> > Test                            HEAD\n>> > ----------------------------------------------------\n>> > 5302.2: index-pack 1 threads    376.88(364.50+11.52)\n>> > 5302.3: index-pack 2 threads    228.13(371.21+17.86)\n>> > 5302.4: index-pack 4 threads    151.41(387.06+21.12)\n>> > 5302.5: index-pack 8 threads    113.68(413.40+25.80)\n>> > 5302.6: index-pack 16 threads   100.60(511.85+37.53)\n>> > 5302.7: index-pack 32 threads   94.43(623.82+45.70)\n>> > 5302.8: index-pack 40 threads   93.64(702.88+47.61)\n>> >\n>> > I don't think any of this is _particularly_ relevant to your case, but\n>> > it really seems to me that the default of capping at 3 threads is too\n>> > low.\n>>\n>> Looking back at the multithread commit, I think the trend was the same\n>> and I capped it because the gain was not proportional to the number of\n>> cores we threw at index-pack anymore. I would not be opposed to\n>> raising the cap though (or maybe just remove it)\n>\n> I'm not sure what the right cap would be. I don't think it's static;\n> we'd want ~4 threads on the top case, and 10-20 on the bottom one.\n>\n> It does seem like there's an inflection point in the graph at N/2\n> threads. But then maybe that's just because these are hyper-threaded\n> machines, so \"N/2\" is the actual number of physical cores, and the\n> inflated CPU times above that are just because we can't turbo-boost\n> then, so we're actually clocking slower. Multi-threaded profiling and\n> measurement is such a mess. :)\n>\n> So I'd say the right answer is probably either online_cpus() or half\n> that. The latter would be more appropriate for the machines I have, but\n> I'd worry that it would leave performance on the table for non-intel\n> machines.\n\nIt would be a nice #leftoverbits project to do this dynamically at\nruntime, i.e. hook up the throughput code in progress.c to some new\nutility functions where the current code using pthreads would\noccasionally stop and try to find some (local) maximum throughput given\nN threads.\n\nYou could then dynamically save that optimum for next time, or adjust\nthreading at runtime every X seconds, e.g. on a server with N=24 cores\nyou might want 24 threads if you have one index-pack, but if you have 24\nindex-packs you probably don't want each with 24 threads, for a total of\n576.\n"},{"id":"374715","messageId":"20190430203353.GA16290@sigill.intra.peff.net","threadId":"50956","inReplyTo":"87sgtzqqhj.fsf@evledraar.gmail.com","subject":"Re: Resolving deltas dominates clone time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-30T20:33:53Z","receivedAt":"2019-04-30T20:33:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 30, 2019 at 08:48:08PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > So I'd say the right answer is probably either online_cpus() or half\n> > that. The latter would be more appropriate for the machines I have, but\n> > I'd worry that it would leave performance on the table for non-intel\n> > machines.\n> \n> It would be a nice #leftoverbits project to do this dynamically at\n> runtime, i.e. hook up the throughput code in progress.c to some new\n> utility functions where the current code using pthreads would\n> occasionally stop and try to find some (local) maximum throughput given\n> N threads.\n> \n> You could then dynamically save that optimum for next time, or adjust\n> threading at runtime every X seconds, e.g. on a server with N=24 cores\n> you might want 24 threads if you have one index-pack, but if you have 24\n> index-packs you probably don't want each with 24 threads, for a total of\n> 576.\n\nYeah, I touched on that in my response to Martin. I think that would be\nnice, but it's complicated enough that I don't think it's a left-over\nbit. I'm also not sure how hard it is to change the number of threads\nafter the initialization.\n\nIIRC, it's a worker pool that just asks for more work. So that's\nprobably the right moment to say not just \"is there more work to do\" but\nalso \"does it seem like there's an idle slot on the system for our\nthread to take\".\n\n-Peff\n"},{"id":"374721","messageId":"7252129.tRx6dl7m8I@mfick-lnx","threadId":"50956","inReplyTo":"20190430180231.GC16729@sigill.intra.peff.net","subject":"Re: Resolving deltas dominates clone time","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2019-04-30T22:08:02Z","receivedAt":"2019-04-30T22:08:07Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Tuesday, April 30, 2019 2:02:32 PM MDT Jeff King wrote:\n> On Tue, Apr 23, 2019 at 02:09:31PM -0600, Martin Fick wrote:\n> > I think that if there were no default limit during a clone it could have\n> > disastrous effects on people using the repo tool from the android project,\n> > or any other \"submodule like\" tool that might clone many projects in\n> > parallel. With the repo tool, people often use a large -j number such as\n> > 24 which means they end up cloning around 24 projects at a time, and they\n> > may do this for around 1000 projects. If git clone suddenly started as\n> > many threads as there are CPUs for each clone, this would likely paralyze\n> > the machine.\n> \n> IMHO this is already a problem, because none of those 24 gits knows\n> about the others. So they're already using 24*3 cores, though of course\n> at any given moment some of those 24 may be bottle-necked on the\n> network.\n\nI think this is very different. In the first case this is a linear constraint \nthat someone can use to intelligently adjust the number of clones they are \ndoing at the same time. The second case cannot be accounted for in any \nintelligent way by the person running the clones, it is almost unconstrained.\n\n...\n> > I do suspect it would be nice to have a switch though that repo could use\n> > to adjust this intelligently, is there some way to adjust threads from a\n> > clone, I don't see one? I tried using 'GIT_FORCE_THREADS=28 git clone\n> > ...' and it didn't seem to make a difference?\n> \n> I think I led you astray earlier by mentioning GIT_FORCE_THREADS. It's\n> actually just a boolean for \"use threads even if we're only\n> single-threaded\". What you actually want is probably:\n> \n>   git clone -c pack.threads=28 ...\n> \n> (though I didn't test it to be sure).\n\nThanks, I will test this!\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n\n"}]}