{"thread":{"id":"42301","subject":"[BUG] t9801 and t9803 broken on next","startedAt":"2016-05-13T10:02:48Z","lastAt":"2016-05-28T00:20:48Z","messageCount":19,"participants":["Lars Schneider","Eric Wong","Junio C Hamano","Luke Diamand","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"286441","messageId":"3A110437-F778-42E2-A987-86B4EAB3D963@gmail.com","threadId":"42301","inReplyTo":null,"subject":"[BUG] t9801 and t9803 broken on next","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-05-13T10:02:48Z","receivedAt":"2016-05-13T10:02:48Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi,\n\nt9801 and t9803 seem to be broken on next. A quick bisect indicates that d9545c7 \n\"fast-import: implement unpack limit\" might be the reason. (@gmane/292562).\n\nDid anyone look into that already?\n\nThanks,\nLars\n"},{"id":"286444","messageId":"20160513103621.GA12329@dcvr.yhbt.net","threadId":"42301","inReplyTo":"3A110437-F778-42E2-A987-86B4EAB3D963@gmail.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-13T10:36:21Z","receivedAt":"2016-05-13T10:36:21Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Lars Schneider wrote:\n> Hi,\n> \n> t9801 and t9803 seem to be broken on next. A quick bisect indicates that d9545c7 \n> \"fast-import: implement unpack limit\" might be the reason. (@gmane/292562).\n> \n> Did anyone look into that already?\n\nJust took a look (no p4, so couldn't run tests) and I guess it's\nbecause the default changed and it doesn't generate tiny packs.\n\nThe easiest change is probably to call:\n\n\tgit config fastimport.unpackLimit 0\n\nduring setup like t9300 now does.\n"},{"id":"286464","messageId":"xmqqpospap8g.fsf@gitster.mtv.corp.google.com","threadId":"42301","inReplyTo":"20160513103621.GA12329@dcvr.yhbt.net","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-13T16:37:19Z","receivedAt":"2016-05-13T16:37:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Lars Schneider wrote:\n>> Hi,\n>> \n>> t9801 and t9803 seem to be broken on next. A quick bisect indicates that d9545c7 \n>> \"fast-import: implement unpack limit\" might be the reason. (@gmane/292562).\n>> \n>> Did anyone look into that already?\n>\n> Just took a look (no p4, so couldn't run tests) and I guess it's\n> because the default changed and it doesn't generate tiny packs.\n\nI looked at t9801 but I couldn't spot where it cares if the objects\nare in a (tiny) pack or stored as individual loose objects. All the\ncounting I saw was about rev-list/log output and they shouldn't care.\n\nAre you saying that \"git p4\" itself breaks unless fast-import always\nwrites a new (tiny) packfile? That sounds quite broken, and setting\nunpacklimit to 0 does not sound like a sensible \"fix\". Of course,\nif the test script is somehow looking at the number of packs or\nloose objects and declaring a failure, even when the resulting\nhistory in p4 and git are correct, then that is a different issue,\nand forcing to explode a tiny pack is a reasonable workaround. I\ncouldn't quite tell which the case is.\n\nPuzzled. Please help.\n\n> The easiest change is probably to call:\n>\n> \tgit config fastimport.unpackLimit 0\n>\n> during setup like t9300 now does.\n"},{"id":"286526","messageId":"4830D469-3885-4010-9A04-D809F0C6159D@gmail.com","threadId":"42301","inReplyTo":"xmqqpospap8g.fsf@gitster.mtv.corp.google.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-05-14T08:04:40Z","receivedAt":"2016-05-14T08:04:40Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 13 May 2016, at 18:37, Junio C Hamano wrote:\n> \n> Eric Wong writes:\n> \n>> Lars Schneider wrote:\n>>> Hi,\n>>> \n>>> t9801 and t9803 seem to be broken on next. A quick bisect indicates that d9545c7 \n>>> \"fast-import: implement unpack limit\" might be the reason. (@gmane/292562).\n>>> \n>>> Did anyone look into that already?\n>> \n>> Just took a look (no p4, so couldn't run tests) and I guess it's\n>> because the default changed and it doesn't generate tiny packs.\n> \n> I looked at t9801 but I couldn't spot where it cares if the objects\n> are in a (tiny) pack or stored as individual loose objects. All the\n> counting I saw was about rev-list/log output and they shouldn't care.\n> \n> Are you saying that \"git p4\" itself breaks unless fast-import always\n> writes a new (tiny) packfile? That sounds quite broken, and setting\n> unpacklimit to 0 does not sound like a sensible \"fix\". Of course,\n> if the test script is somehow looking at the number of packs or\n> loose objects and declaring a failure, even when the resulting\n> history in p4 and git are correct, then that is a different issue,\n> and forcing to explode a tiny pack is a reasonable workaround. I\n> couldn't quite tell which the case is.\n> \n> Puzzled. Please help.\n\nt9801 \"import depot, branch detection\" is the first test that fails\nwith a fast import error:\nhttps://github.com/git/git/blob/78b384c29366e199741393e56030a8384110760d/t/t9801-git-p4-branch.sh#L110\n\nfast-import crash report:\n fast-import process: 77079\n parent process : 77077\n at 2016-05-14 07:48:40 +0000\n\nfatal: offset beyond end of packfile (truncated pack?)\n\nMost Recent Commands Before Crash\n---------------------------------\n commit refs/remotes/p4/master\n mark :1\n committer Dr. author 1463212118 +0000\n data < 1463212118 +0000\n data < 1463212118 +0000\n data < 1463212118 +0000\n data < 1463212118 +0000\n data < \n>> The easiest change is probably to call:\n>> \n>> \tgit config fastimport.unpackLimit 0\n>> \n>> during setup like t9300 now does.\nUnfortunately that seems not to help.\n\nThe problem seems to be related to the git-p4 branch import.\nI don't have time this weekend to look further into it, but\nI will try next week.\n\nCheers,\nLars"},{"id":"286541","messageId":"xmqq37pk8q0h.fsf@gitster.mtv.corp.google.com","threadId":"42301","inReplyTo":"4830D469-3885-4010-9A04-D809F0C6159D@gmail.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-14T18:15:42Z","receivedAt":"2016-05-14T18:15:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> On 13 May 2016, at 18:37, Junio C Hamano wrote:\n>> \n>> Are you saying that \"git p4\" itself breaks unless fast-import always\n>> writes a new (tiny) packfile? That sounds quite broken, and setting\n>> unpacklimit to 0 does not sound like a sensible \"fix\". Of course,\n>> if the test script is somehow looking at the number of packs or\n>> loose objects and declaring a failure, even when the resulting\n>> history in p4 and git are correct, then that is a different issue,\n>> and forcing to explode a tiny pack is a reasonable workaround. I\n>> couldn't quite tell which the case is.\n>> \n>> Puzzled. Please help.\n>\n> t9801 \"import depot, branch detection\" is the first test that fails\n> with a fast import error:\n> https://github.com/git/git/blob/78b384c29366e199741393e56030a8384110760d/t/t9801-git-p4-branch.sh#L110\n>\n> fast-import crash report:\n> fast-import process: 77079\n> parent process : 77077\n> at 2016-05-14 07:48:40 +0000\n>\n> fatal: offset beyond end of packfile (truncated pack?)\n\nHmm, does that suggest Eric's \"explode them into loose instead of\nkeeping a small pack\" insufficient? It sounds like that somebody\nwanted to read some data back from its packfile without knowing that\nthe updated code may make it available in a loose object form, which\nwould mean that somebody needs to be told about what is going on,\nnamely, these objects are not in a half-written pack but are found\nas loose objects.\n\n> The problem seems to be related to the git-p4 branch import.\n> I don't have time this weekend to look further into it, but\n> I will try next week.\n\nThanks.\n"},{"id":"286781","messageId":"5E7631C9-DD59-4358-B907-D7C7AEA1739C@gmail.com","threadId":"42301","inReplyTo":"xmqq37pk8q0h.fsf@gitster.mtv.corp.google.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-05-17T08:07:16Z","receivedAt":"2016-05-17T08:07:16Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 14 May 2016, at 20:15, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>>> On 13 May 2016, at 18:37, Junio C Hamano wrote:\n>>> \n>>> Are you saying that \"git p4\" itself breaks unless fast-import always\n>>> writes a new (tiny) packfile? That sounds quite broken, and setting\n>>> unpacklimit to 0 does not sound like a sensible \"fix\". Of course,\n>>> if the test script is somehow looking at the number of packs or\n>>> loose objects and declaring a failure, even when the resulting\n>>> history in p4 and git are correct, then that is a different issue,\n>>> and forcing to explode a tiny pack is a reasonable workaround. I\n>>> couldn't quite tell which the case is.\n>>> \n>>> Puzzled. Please help.\n>> \n>> t9801 \"import depot, branch detection\" is the first test that fails\n>> with a fast import error:\n>> https://github.com/git/git/blob/78b384c29366e199741393e56030a8384110760d/t/t9801-git-p4-branch.sh#L110\n>> \n>> fast-import crash report:\n>> fast-import process: 77079\n>> parent process : 77077\n>> at 2016-05-14 07:48:40 +0000\n>> \n>> fatal: offset beyond end of packfile (truncated pack?)\n> \n> Hmm, does that suggest Eric's \"explode them into loose instead of\n> keeping a small pack\" insufficient? It sounds like that somebody\n> wanted to read some data back from its packfile without knowing that\n> the updated code may make it available in a loose object form, which\n> would mean that somebody needs to be told about what is going on,\n> namely, these objects are not in a half-written pack but are found\n> as loose objects.\n\nI think that is pretty much the problem. Here is what is happening:\n\n1. git-p4 imports all changelists for the \"main\" branch\n\n2. git-p4 starts to import a second branch and creates a fast-import\n \"progress checkpoint\". This triggers:\n\n --> fast-import.c: cycle_packfile\n --> fast-import.c: end_packfile\n --> fast-import.c: loosen_small_pack\n\n At this point we have no packfile anymore.\n\n3. git-p4 sends the command \"commit refs/remotes/p4/depot/branch2\"\n to fast-import in order to create a new branch. This triggers:\n\n --> fast-import.c: parse_new_commit\n --> fast-import.c: load_branch\n --> fast-import.c: load_tree\n\n \"load_tree\" calls \"find_object\" and the result has a \"pack_id\" of 0.\n I think this indicates that the object is in the packfile. Shouldn't\n the \"pack_id\" be \"MAX_PACK_ID\" instead?\n\n myoe = find_object(sha1);\n if (myoe && myoe->pack_id != MAX_PACK_ID) {\n\n --> fast-import.c: gfi_unpack_entry\n\n In \"gfi_unpack_entry\" this condition evaluates to \"true\":\n\n struct packed_git *p = all_packs[oe->pack_id];\n if (p == pack_data && p->pack_size < (pack_size + 20)) \n\n In the comment below Git thinks that the object is stored in the\n packfile. This seems wrong but the flow continues:\n\n --> sha1_filec: unpack_entry\n --> sha1_filec: unpack_object_header\n --> sha1_filec: use_pack\n\n At this point fast-import dies with \"offset beyond end of packfile \n (truncated pack?)\".\n\nI am no expert in \"fast-import\". Does anyone have a few hints how to\nproceed?\n\nThanks,\nLars"},{"id":"286782","messageId":"CAE5ih79qUv237GXfW24HdJrx0WBiK4uvuWXe+d3v7NiOVtwHGQ@mail.gmail.com","threadId":"42301","inReplyTo":"xmqq37pk8q0h.fsf@gitster.mtv.corp.google.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-05-17T08:35:57Z","receivedAt":"2016-05-17T08:35:57Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 14 May 2016 at 19:15, Junio C Hamano wrote:\n> Lars Schneider writes:\n>\n>>> On 13 May 2016, at 18:37, Junio C Hamano wrote:\n>>>\n>>> Are you saying that \"git p4\" itself breaks unless fast-import always\n>>> writes a new (tiny) packfile? That sounds quite broken, and setting\n>>> unpacklimit to 0 does not sound like a sensible \"fix\". Of course,\n>>> if the test script is somehow looking at the number of packs or\n>>> loose objects and declaring a failure, even when the resulting\n>>> history in p4 and git are correct, then that is a different issue,\n>>> and forcing to explode a tiny pack is a reasonable workaround. I\n>>> couldn't quite tell which the case is.\n>>>\n>>> Puzzled. Please help.\n>>\n>> t9801 \"import depot, branch detection\" is the first test that fails\n>> with a fast import error:\n>> https://github.com/git/git/blob/78b384c29366e199741393e56030a8384110760d/t/t9801-git-p4-branch.sh#L110\n>>\n>> fast-import crash report:\n>> fast-import process: 77079\n>> parent process : 77077\n>> at 2016-05-14 07:48:40 +0000\n>>\n>> fatal: offset beyond end of packfile (truncated pack?)\n>\n> Hmm, does that suggest Eric's \"explode them into loose instead of\n> keeping a small pack\" insufficient? It sounds like that somebody\n> wanted to read some data back from its packfile without knowing that\n> the updated code may make it available in a loose object form, which\n> would mean that somebody needs to be told about what is going on,\n> namely, these objects are not in a half-written pack but are found\n> as loose objects.\n>\n>> The problem seems to be related to the git-p4 branch import.\n>> I don't have time this weekend to look further into it, but\n>> I will try next week.\n\nThe last thing that fast-import is sent before it crashes is this:\n\ncheckpoint\n progress checkpoint\n commit refs/remotes/p4/depot/branch2\n mark :6\n committer Dr. author 1463212118 +0000\n data < wrote:\n> I am no expert in \"fast-import\". Does anyone have a few hints how to\n> proceed?\n\nI don't know fast-import or the C internals of git well at all,\neither. But capturing the exact input to fast-import (using\ntee) would be useful for non-p4 users to debug the problem\nand would go a long way in reproducing a standalone test case.\n\nI'm Python-illiterate , but hopefully this works\nto capture the stdin fed to fast-import (totally untested):\n\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3366,7 +3366,8 @@ class P4Sync(Command, P4UserMap):\n \n self.tz = \"%+03d%02d\" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))\n \n- self.importProcess = subprocess.Popen([\"git\", \"fast-import\"],\n+ cmd = [ \"sh\", \"-c\", \"tee /tmp/gfi-in.$$ | git fast-import\" ]\n+ self.importProcess = subprocess.Popen(cmd,\n stdin=subprocess.PIPE,\n stdout=subprocess.PIPE,\n stderr=subprocess.PIPE);\n"},{"id":"286787","messageId":"20160517121330.GA7346@sigill.intra.peff.net","threadId":"42301","inReplyTo":"5E7631C9-DD59-4358-B907-D7C7AEA1739C@gmail.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-17T12:13:31Z","receivedAt":"2016-05-17T12:13:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 17, 2016 at 10:07:16AM +0200, Lars Schneider wrote:\n\n> I think that is pretty much the problem. Here is what is happening:\n> \n> 1. git-p4 imports all changelists for the \"main\" branch\n> \n> 2. git-p4 starts to import a second branch and creates a fast-import\n> \"progress checkpoint\". This triggers:\n> \n> --> fast-import.c: cycle_packfile\n> --> fast-import.c: end_packfile\n> --> fast-import.c: loosen_small_pack\n> \n> At this point we have no packfile anymore.\n> \n> 3. git-p4 sends the command \"commit refs/remotes/p4/depot/branch2\"\n> to fast-import in order to create a new branch. This triggers:\n> \n> --> fast-import.c: parse_new_commit\n> --> fast-import.c: load_branch\n> --> fast-import.c: load_tree\n> \n> \"load_tree\" calls \"find_object\" and the result has a \"pack_id\" of 0.\n> I think this indicates that the object is in the packfile. Shouldn't\n> the \"pack_id\" be \"MAX_PACK_ID\" instead?\n> \n> myoe = find_object(sha1);\n> if (myoe && myoe->pack_id != MAX_PACK_ID) {\n\nThanks for the analysis. I think this is definitely the problem. After\nfast-import finalizes a packfile (either at the end of the program or\ndue to a checkpoint), it never discards its internal mapping of objects\nto pack locations. It _has_ to keep such a mapping before the pack is\nfinalized, because git's regular object database doesn't know about it.\nBut afterwards, it should be able to rely on the object database.\n\nRetaining the mapping at the end of the program is obviously OK because\nwe won't make any more lookups in it.\n\nRetaining it at a checkpoint when we keep the packfile is OK, because we\ncan (usually) still access that packfile (the exception would be if\nsomebody runs \"git repack\" while fast-import is running).\n\nBut obviously a checkpoint where we throw away the pack needs to clear\nthat mapping.\n\nThe patch below probably makes your case work, but there are a lot of\nopen questions:\n\n 1. Should we always discard the mapping, even when not loosening\n objects? I can't think of a real downside to always using git's\n object lookups.\n\n 2. Can we free memory associated with object_entry structs at this\n point? They won't be accessible via the hash, but might other bits\n of the code have kept pointers to them?\n\n I suspect it may screw up the statistics that fast-import prints at\n the end, but that's a minor point.\n\n 3. I notice that a few other structs (branch and tag) hold onto the\n pack_id, which will now point to a pack we can't access. Does this\n matter? I don't think so, because checkpoint() seems to dump the\n branches and tags.\n\n 4. In general, should we be loosening objects at checkpoints at all?\n\n I guess it is probably more efficient than creating a bunch of\n little packs. And it should probably not happen much at all outside\n of tests (i.e., callers should generally checkpoint after an\n appreciable amount of work is done).\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 0e8bc6a..9bfbfb0 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -597,6 +597,22 @@ static struct object_entry *insert_object(unsigned char *sha1)\n \treturn e;\n }\n \n+static void clear_object_table(void)\n+{\n+\tunsigned int h;\n+\tfor (h = 0; h < ARRAY_SIZE(object_table); h++) {\n+\t\t/*\n+\t\t * We can't individually free objects here\n+\t\t * because they are allocated from a pool.\n+\t\t */\n+\t\tobject_table[h] = NULL;\n+\t}\n+\t/*\n+\t * XXX maybe free object_entry_pool here,\n+\t * or might something still be referencing them?\n+\t */\n+}\n+\n static unsigned int hc_str(const char *s, size_t len)\n {\n \tunsigned int r = 0;\n@@ -1035,6 +1051,9 @@ discard_pack:\n \tpack_data = NULL;\n \trunning = 0;\n \n+\t/* The objects are now available via git's regular lookups. */\n+\tclear_object_table();\n+\n \t/* We can't carry a delta across packfiles. */\n \tstrbuf_release(&last_blob.data);\n \tlast_blob.offset = 0;\n"},{"id":"286965","messageId":"DD9F360B-78F0-4C83-B642-606688772E39@gmail.com","threadId":"42301","inReplyTo":"20160517121330.GA7346@sigill.intra.peff.net","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-05-19T08:19:51Z","receivedAt":"2016-05-19T08:19:51Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 17 May 2016, at 14:13, Jeff King wrote:\n\n> On Tue, May 17, 2016 at 10:07:16AM +0200, Lars Schneider wrote:\n> \n>> I think that is pretty much the problem. Here is what is happening:\n>> \n>> 1. git-p4 imports all changelists for the \"main\" branch\n>> \n>> 2. git-p4 starts to import a second branch and creates a fast-import\n>> \"progress checkpoint\". This triggers:\n>> \n>> --> fast-import.c: cycle_packfile\n>> --> fast-import.c: end_packfile\n>> --> fast-import.c: loosen_small_pack\n>> \n>> At this point we have no packfile anymore.\n>> \n>> 3. git-p4 sends the command \"commit refs/remotes/p4/depot/branch2\"\n>> to fast-import in order to create a new branch. This triggers:\n>> \n>> --> fast-import.c: parse_new_commit\n>> --> fast-import.c: load_branch\n>> --> fast-import.c: load_tree\n>> \n>> \"load_tree\" calls \"find_object\" and the result has a \"pack_id\" of 0.\n>> I think this indicates that the object is in the packfile. Shouldn't\n>> the \"pack_id\" be \"MAX_PACK_ID\" instead?\n>> \n>> myoe = find_object(sha1);\n>> if (myoe && myoe->pack_id != MAX_PACK_ID) {\n> \n> Thanks for the analysis. I think this is definitely the problem. After\n> fast-import finalizes a packfile (either at the end of the program or\n> due to a checkpoint), it never discards its internal mapping of objects\n> to pack locations. It _has_ to keep such a mapping before the pack is\n> finalized, because git's regular object database doesn't know about it.\n> But afterwards, it should be able to rely on the object database.\n> \n> Retaining the mapping at the end of the program is obviously OK because\n> we won't make any more lookups in it.\n> \n> Retaining it at a checkpoint when we keep the packfile is OK, because we\n> can (usually) still access that packfile (the exception would be if\n> somebody runs \"git repack\" while fast-import is running).\n> \n> But obviously a checkpoint where we throw away the pack needs to clear\n> that mapping.\n> \n> The patch below probably makes your case work, but there are a lot of\n> open questions:\nConfirmed. The offending tests pass with your patch.\n\n\n> 1. Should we always discard the mapping, even when not loosening\n> objects? I can't think of a real downside to always using git's\n> object lookups.\n> \n> 2. Can we free memory associated with object_entry structs at this\n> point? They won't be accessible via the hash, but might other bits\n> of the code have kept pointers to them?\n> \n> I suspect it may screw up the statistics that fast-import prints at\n> the end, but that's a minor point.\n> \n> 3. I notice that a few other structs (branch and tag) hold onto the\n> pack_id, which will now point to a pack we can't access. Does this\n> matter? I don't think so, because checkpoint() seems to dump the\n> branches and tags.\n> 4. In general, should we be loosening objects at checkpoints at all?\n> \n> I guess it is probably more efficient than creating a bunch of\n> little packs. And it should probably not happen much at all outside\n> of tests (i.e., callers should generally checkpoint after an\n> appreciable amount of work is done).\nI am not too familiar with the code and therefore I can't give a good\nanswer to your questions. However, I noticed that Shawn (CC'ed) wrote a \nlot of fast-import code. Maybe he can help?\n\n>From my point of view little packs are no problem. I run fast-import on\na dedicated migration machine. After fast-import completion I run repack [1] \nbefore I upload the repo to its final location.\n\n\nThanks,\nLars\n\n[1] https://gcc.gnu.org/ml/gcc/2007-12/msg00165.html\n\n\n> \n> diff --git a/fast-import.c b/fast-import.c\n> index 0e8bc6a..9bfbfb0 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -597,6 +597,22 @@ static struct object_entry *insert_object(unsigned char *sha1)\n> \treturn e;\n> }\n> \n> +static void clear_object_table(void)\n> +{\n> +\tunsigned int h;\n> +\tfor (h = 0; h < ARRAY_SIZE(object_table); h++) {\n> +\t\t/*\n> +\t\t * We can't individually free objects here\n> +\t\t * because they are allocated from a pool.\n> +\t\t */\n> +\t\tobject_table[h] = NULL;\n> +\t}\n> +\t/*\n> +\t * XXX maybe free object_entry_pool here,\n> +\t * or might something still be referencing them?\n> +\t */\n> +}\n> +\n> static unsigned int hc_str(const char *s, size_t len)\n> {\n> \tunsigned int r = 0;\n> @@ -1035,6 +1051,9 @@ discard_pack:\n> \tpack_data = NULL;\n> \trunning = 0;\n> \n> +\t/* The objects are now available via git's regular lookups. */\n> +\tclear_object_table();\n> +\n> \t/* We can't carry a delta across packfiles. */\n> \tstrbuf_release(&last_blob.data);\n> \tlast_blob.offset = 0;\n"},{"id":"286996","messageId":"xmqq1t4ykmjj.fsf@gitster.mtv.corp.google.com","threadId":"42301","inReplyTo":"DD9F360B-78F0-4C83-B642-606688772E39@gmail.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-19T17:03:28Z","receivedAt":"2016-05-19T17:03:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> From my point of view little packs are no problem. I run fast-import on\n> a dedicated migration machine. After fast-import completion I run repack [1] \n> before I upload the repo to its final location.\n\nHow do you determine that many little packs are not a problem to\nyou, though? Until they get repacked, they are where the\nfast-import will get existing objects from. The more little packs\nyou accumulate as you go, the more slow fast-import's access to them\nhas to become.\n"},{"id":"287000","messageId":"9CA2E5C1-ECAD-4C8C-A495-55041A1CC256@gmail.com","threadId":"42301","inReplyTo":"xmqq1t4ykmjj.fsf@gitster.mtv.corp.google.com","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-05-19T17:32:59Z","receivedAt":"2016-05-19T17:32:59Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 19 May 2016, at 19:03, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>> From my point of view little packs are no problem. I run fast-import on\n>> a dedicated migration machine. After fast-import completion I run repack [1] \n>> before I upload the repo to its final location.\n> \n> How do you determine that many little packs are not a problem to\n> you, though? Until they get repacked, they are where the\n> fast-import will get existing objects from. The more little packs\n> you accumulate as you go, the more slow fast-import's access to them\n> has to become.\nTrue. But my particular use case is a one time operation for a given\nrepository. Therefore I don't care how long it takes. The only important\naspect is that the result is correct.\n\nThat being said, Peff's proposed fix looks correct to me but since I \nam no fast-import expert my opinion doesn't count too much.\n\n- Lars\n"},{"id":"287529","messageId":"20160525224907.GA18894@dcvr.yhbt.net","threadId":"42301","inReplyTo":"20160517121330.GA7346@sigill.intra.peff.net","subject":"Re: [BUG] t9801 and t9803 broken on next","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-25T22:49:07Z","receivedAt":"2016-05-25T22:49:07Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King wrote:\n> On Tue, May 17, 2016 at 10:07:16AM +0200, Lars Schneider wrote:\n> \n> > I think that is pretty much the problem. Here is what is happening:\n> > \n> > 1. git-p4 imports all changelists for the \"main\" branch\n> > \n> > 2. git-p4 starts to import a second branch and creates a fast-import\n> > \"progress checkpoint\". This triggers:\n> > \n> > --> fast-import.c: cycle_packfile\n> > --> fast-import.c: end_packfile\n> > --> fast-import.c: loosen_small_pack\n> > \n> > At this point we have no packfile anymore.\n> > \n> > 3. git-p4 sends the command \"commit refs/remotes/p4/depot/branch2\"\n> > to fast-import in order to create a new branch. This triggers:\n> > \n> > --> fast-import.c: parse_new_commit\n> > --> fast-import.c: load_branch\n> > --> fast-import.c: load_tree\n> > \n> > \"load_tree\" calls \"find_object\" and the result has a \"pack_id\" of 0.\n> > I think this indicates that the object is in the packfile. Shouldn't\n> > the \"pack_id\" be \"MAX_PACK_ID\" instead?\n\nYes; I think that is correct. Alternative patch to Jeff's\ncoming in reply to this message.\n\n> > myoe = find_object(sha1);\n> > if (myoe && myoe->pack_id != MAX_PACK_ID) {\n> \n> Thanks for the analysis. I think this is definitely the problem. After\n> fast-import finalizes a packfile (either at the end of the program or\n> due to a checkpoint), it never discards its internal mapping of objects\n> to pack locations. It _has_ to keep such a mapping before the pack is\n> finalized, because git's regular object database doesn't know about it.\n> But afterwards, it should be able to rely on the object database.\n\nAlmost; but relying on marks is a problem since that set can contain\nmark => object_entry mappings which the normal object database won't\nknow about.\n\n> The patch below probably makes your case work, but there are a lot of\n> open questions:\n> \n> 1. Should we always discard the mapping, even when not loosening\n> objects? I can't think of a real downside to always using git's\n> object lookups.\n\nI'm not sure. It's safe to clear the top-level table, but it\nmight speedup some lookups for just oe->type if we keep it\naround.\n\nI decided to keep it, anyways, because the mark set references them.\n\n> 2. Can we free memory associated with object_entry structs at this\n> point? They won't be accessible via the hash, but might other bits\n> of the code have kept pointers to them?\n\nYes, invalid entries are also held in \"struct mark_set marks\";\nthis is a major problem with merely clearing the top-level\nobject table.\n\n> I suspect it may screw up the statistics that fast-import prints at\n> the end, but that's a minor point.\n\nI still need to check, on that; but yeah, minor.\n\n> 3. I notice that a few other structs (branch and tag) hold onto the\n> pack_id, which will now point to a pack we can't access. Does this\n> matter? I don't think so, because checkpoint() seems to dump the\n> branches and tags.\n\nI don't think it matters unless a crash log or core dump is\ncreated; then it becomes confusing to the person tracking down a\nproblem, so I've invalidated pack_id. This doesn't affect\ndump_branches or dump_tags from what I can tell.\n\n> 4. In general, should we be loosening objects at checkpoints at all?\n\nI think so. It should be useful to checkpoint to make objects\navailable to other read-only processes while leaving a\nfast-import running indefinitely.\n"},{"id":"287531","messageId":"20160525225402.GB18894@dcvr.yhbt.net","threadId":"42301","inReplyTo":"20160525224907.GA18894@dcvr.yhbt.net","subject":"[RFC] fast-import: invalidate pack_id references after loosening","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-25T22:54:02Z","receivedAt":"2016-05-25T22:54:02Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"When loosening a pack, the current pack_id gets reused when\ncheckpointing and the import does not terminate. This causes\nproblems after checkpointing as the object table, branch, and\ntag lists still contains pre-checkpoint references to the\nrecycled pack_id.\n\nMerely clearing the object_table as suggested by Jeff King in\nhttp://mid.gmane.org/20160517121330.GA7346@sigill.intra.peff.net\nis insufficient as the marks set still contains references\nto object entries.\n\nWrong pack_id references branch and tags lists do not cause\nerrors, but can lead to misleading crash reports and core dumps,\nso they are also invalidated.\n\nSigned-off-by: Eric Wong \n---\n I started writing a standalone test case; but testing with\n original failing cases would be greatly appreciated.\n\n Still learning my way around the fast-import code...\n Thanks.\n\n fast-import.c | 31 +++++++++++++++++++-\n t/t9302-fast-import-unpack-limit.sh | 57 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 87 insertions(+), 1 deletion(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 0e8bc6a..b9db4b6 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -597,6 +597,33 @@ static struct object_entry *insert_object(unsigned char *sha1)\n \treturn e;\n }\n \n+static void invalidate_pack_id(unsigned int id)\n+{\n+\tunsigned int h;\n+\tunsigned long lu;\n+\tstruct tag *t;\n+\n+\tfor (h = 0; h < ARRAY_SIZE(object_table); h++) {\n+\t\tstruct object_entry *e;\n+\n+\t\tfor (e = object_table[h]; e; e = e->next)\n+\t\t\tif (e->pack_id == id)\n+\t\t\t\te->pack_id = MAX_PACK_ID;\n+\t}\n+\n+\tfor (lu = 0; lu < branch_table_sz; lu++) {\n+\t\tstruct branch *b;\n+\n+\t\tfor (b = branch_table[lu]; b; b = b->table_next_branch)\n+\t\t\tif (b->pack_id == id)\n+\t\t\t\tb->pack_id = MAX_PACK_ID;\n+\t}\n+\n+\tfor (t = first_tag; t; t = t->next_tag)\n+\t\tif (t->pack_id == id)\n+\t\t\tt->pack_id = MAX_PACK_ID;\n+}\n+\n static unsigned int hc_str(const char *s, size_t len)\n {\n \tunsigned int r = 0;\n@@ -993,8 +1020,10 @@ static void end_packfile(void)\n \t\t\t\t cur_pack_sha1, pack_size);\n \n \t\tif (object_count <= unpack_limit) {\n-\t\t\tif (!loosen_small_pack(pack_data))\n+\t\t\tif (!loosen_small_pack(pack_data)) {\n+\t\t\t\tinvalidate_pack_id(pack_id);\n \t\t\t\tgoto discard_pack;\n+\t\t\t}\n \t\t}\n \n \t\tclose(pack_data->pack_fd);\ndiff --git a/t/t9302-fast-import-unpack-limit.sh b/t/t9302-fast-import-unpack-limit.sh\nindex 0f686d2..a04de14 100755\n--- a/t/t9302-fast-import-unpack-limit.sh\n+++ b/t/t9302-fast-import-unpack-limit.sh\n@@ -45,4 +45,61 @@ test_expect_success 'bigger packs are preserved' '\n \ttest $(find .git/objects/pack -type f | wc -l) -eq 2\n '\n \n+test_expect_success 'lookups after checkpoint works' '\n+\thello_id=$(echo hello | git hash-object --stdin -t blob) &&\n+\tid=\"$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\" &&\n+\tbefore=$(git rev-parse refs/heads/master^0) &&\n+\t(\n+\t\tcat <<-INPUT_END &&\n+\t\tblob\n+\t\tmark :1\n+\t\tdata 6\n+\t\thello\n+\n+\t\tcommit refs/heads/master\n+\t\tmark :2\n+\t\tcommitter $id\n+\t\tdata <&2 \"checkpoint did not update branch\"\n+\t\t\t\texit 1\n+\t\t\telse\n+\t\t\t\tn=$(($n + 1))\n+\t\t\tfi &&\n+\t\t\tsleep 1 &&\n+\t\t\tfrom=$(git rev-parse refs/heads/master^0)\n+\t\tdone &&\n+\t\tcat <<-INPUT_END &&\n+\t\tcommit refs/heads/master\n+\t\tcommitter $id\n+\t\tdata < > Thanks for the analysis. I think this is definitely the problem. After\n> > fast-import finalizes a packfile (either at the end of the program or\n> > due to a checkpoint), it never discards its internal mapping of objects\n> > to pack locations. It _has_ to keep such a mapping before the pack is\n> > finalized, because git's regular object database doesn't know about it.\n> > But afterwards, it should be able to rely on the object database.\n> \n> Almost; but relying on marks is a problem since that set can contain\n> mark => object_entry mappings which the normal object database won't\n> know about.\n\nAh, thanks for finding that. I had a feeling there might be other users\nof the object_entry structs, but I didn't dig.\n\nGiven that and your other responses here, I agree that just invalidating\nthe pack_id is probably the most sensible route.\n\n-Peff\n"},{"id":"287534","messageId":"20160525230913.GB2634@sigill.intra.peff.net","threadId":"42301","inReplyTo":"20160525225402.GB18894@dcvr.yhbt.net","subject":"Re: [RFC] fast-import: invalidate pack_id references after loosening","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-25T23:09:13Z","receivedAt":"2016-05-25T23:09:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 25, 2016 at 10:54:02PM +0000, Eric Wong wrote:\n\n> When loosening a pack, the current pack_id gets reused when\n> checkpointing and the import does not terminate. This causes\n> problems after checkpointing as the object table, branch, and\n> tag lists still contains pre-checkpoint references to the\n> recycled pack_id.\n> \n> Merely clearing the object_table as suggested by Jeff King in\n> http://mid.gmane.org/20160517121330.GA7346@sigill.intra.peff.net\n> is insufficient as the marks set still contains references\n> to object entries.\n> \n> Wrong pack_id references branch and tags lists do not cause\n> errors, but can lead to misleading crash reports and core dumps,\n> so they are also invalidated.\n\nNicely explained.\n\n> +static void invalidate_pack_id(unsigned int id)\n> +{\n> +\tunsigned int h;\n> +\tunsigned long lu;\n> +\tstruct tag *t;\n> +\n> +\tfor (h = 0; h < ARRAY_SIZE(object_table); h++) {\n> +\t\tstruct object_entry *e;\n> +\n> +\t\tfor (e = object_table[h]; e; e = e->next)\n> +\t\t\tif (e->pack_id == id)\n> +\t\t\t\te->pack_id = MAX_PACK_ID;\n> +\t}\n> +\n> +\tfor (lu = 0; lu < branch_table_sz; lu++) {\n> +\t\tstruct branch *b;\n> +\n> +\t\tfor (b = branch_table[lu]; b; b = b->table_next_branch)\n> +\t\t\tif (b->pack_id == id)\n> +\t\t\t\tb->pack_id = MAX_PACK_ID;\n> +\t}\n> +\n> +\tfor (t = first_tag; t; t = t->next_tag)\n> +\t\tif (t->pack_id == id)\n> +\t\t\tt->pack_id = MAX_PACK_ID;\n> +}\n\nThis looks pretty straightforward. I do notice that we never shrink the\nnumber of items in the object table when checkpointing, and so our\nlinear walk will grow ever larger. So if one were to checkpoint every\nk-th object, it makes the whole operation quadratic in the number of\nobjects (actually, O(n^2/k) but k is a constant).\n\nThat's probably OK in practice, as I think the actual pack-write already\ndoes a linear walk of the object table to generate the pack index. So\npresumably nobody checkpoints often enough for it to be a problem. And\nthe solution would be to keep a separate list of pointers to objects for\nthe current pack-id, which would trivially fix both this case and the\none in create_index(). So we can punt on it until somebody actually\nruns into it, I think.\n\n-Peff\n"},{"id":"287558","messageId":"20160526080236.GA27652@dcvr.yhbt.net","threadId":"42301","inReplyTo":"20160525230913.GB2634@sigill.intra.peff.net","subject":"Re: [RFC] fast-import: invalidate pack_id references after loosening","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-26T08:02:36Z","receivedAt":"2016-05-26T08:02:36Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King wrote:\n> On Wed, May 25, 2016 at 10:54:02PM +0000, Eric Wong wrote:\n> > +\tfor (h = 0; h < ARRAY_SIZE(object_table); h++) {\n> > +\t\tstruct object_entry *e;\n> > +\n> > +\t\tfor (e = object_table[h]; e; e = e->next)\n> > +\t\t\tif (e->pack_id == id)\n> > +\t\t\t\te->pack_id = MAX_PACK_ID;\n> > +\t}\n\n\n\n> This looks pretty straightforward. I do notice that we never shrink the\n> number of items in the object table when checkpointing, and so our\n> linear walk will grow ever larger. So if one were to checkpoint every\n> k-th object, it makes the whole operation quadratic in the number of\n> objects (actually, O(n^2/k) but k is a constant).\n\nGood point, I'll work on a separate patch to fix it.\n\n> That's probably OK in practice, as I think the actual pack-write already\n> does a linear walk of the object table to generate the pack index. So\n> presumably nobody checkpoints often enough for it to be a problem. And\n> the solution would be to keep a separate list of pointers to objects for\n> the current pack-id, which would trivially fix both this case and the\n> one in create_index(). So we can punt on it until somebody actually\n> runs into it, I think.\n\nI might checkpoint that much and run into the problem soon :)\nToo tired now; maybe in a day or two I'll be able to make sense\nof C again :x\n"},{"id":"287576","messageId":"20160526141644.GA15836@sigill.intra.peff.net","threadId":"42301","inReplyTo":"20160526080236.GA27652@dcvr.yhbt.net","subject":"Re: [RFC] fast-import: invalidate pack_id references after loosening","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-26T14:16:45Z","receivedAt":"2016-05-26T14:16:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 26, 2016 at 08:02:36AM +0000, Eric Wong wrote:\n\n> > That's probably OK in practice, as I think the actual pack-write already\n> > does a linear walk of the object table to generate the pack index. So\n> > presumably nobody checkpoints often enough for it to be a problem. And\n> > the solution would be to keep a separate list of pointers to objects for\n> > the current pack-id, which would trivially fix both this case and the\n> > one in create_index(). So we can punt on it until somebody actually\n> > runs into it, I think.\n> \n> I might checkpoint that much and run into the problem soon :)\n> Too tired now; maybe in a day or two I'll be able to make sense\n> of C again :x\n\nIn theory the list of objects in the allocation pool are contiguous for\na particular pack. So you could break out of the double-loop at the\nfirst non-matching object you see:\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 83558dc..f214bd3 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -900,10 +900,13 @@ static const char *create_index(void)\n \tc = idx;\n \tfor (o = blocks; o; o = o->next_pool)\n \t\tfor (e = o->next_free; e-- != o->entries;)\n \t\t\tif (pack_id == e->pack_id)\n \t\t\t\t*c++ = &e->idx;\n+\t\t\telse\n+\t\t\t\tgoto done;\n+done:\n \tlast = idx + object_count;\n \tif (c != last)\n \t\tdie(\"internal consistency error creating the index\");\n \n \ttmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts, pack_data->sha1);\n\nBut that seems to break some of the tests, so I think there is some case\nwhich does not match my theory. :)\n\nAnother strategy is to note that we cross-check against object_count\nhere. So you could simply break out of the loop when we have found\nobject_count matches.\n\nWe don't keep similar counters for branches/tags (which have a similar\nquadratic problem, though presumably much smaller \"n\"), but it would be\neasy to keep an offset in start_packfile() and just start the iteration\nthere.\n\n-Peff\n"},{"id":"287720","messageId":"20160528002048.GA31463@dcvr.yhbt.net","threadId":"42301","inReplyTo":"20160526141644.GA15836@sigill.intra.peff.net","subject":"Re: [RFC] fast-import: invalidate pack_id references after loosening","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-28T00:20:48Z","receivedAt":"2016-05-28T00:20:48Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King wrote:\n> On Thu, May 26, 2016 at 08:02:36AM +0000, Eric Wong wrote:\n> \n> > > That's probably OK in practice, as I think the actual pack-write already\n> > > does a linear walk of the object table to generate the pack index. So\n> > > presumably nobody checkpoints often enough for it to be a problem. And\n> > > the solution would be to keep a separate list of pointers to objects for\n> > > the current pack-id, which would trivially fix both this case and the\n> > > one in create_index(). So we can punt on it until somebody actually\n> > > runs into it, I think.\n> > \n> > I might checkpoint that much and run into the problem soon :)\n> > Too tired now; maybe in a day or two I'll be able to make sense\n> > of C again :x\n\nOK, I guess I was too tired and not thinking clearly.\n\nI now figure it's not worth it to checkpoint frequently and have\na very-long-lived fast-import process. The object table and\nmarks set will grow indefinitely, so forking off another gfi\nwith a new set of marks is better for my case[1], anyways.\n\n\nJunio: I think my RFC can go into pu as-is and replace\nJeff's object_table discarding patch.\n\n\n[1] inotify watching a Maildir + fast-import\n\n> In theory the list of objects in the allocation pool are contiguous for\n> a particular pack. So you could break out of the double-loop at the\n> first non-matching object you see:\n> \n> diff --git a/fast-import.c b/fast-import.c\n> index 83558dc..f214bd3 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -900,10 +900,13 @@ static const char *create_index(void)\n> \tc = idx;\n> \tfor (o = blocks; o; o = o->next_pool)\n> \t\tfor (e = o->next_free; e-- != o->entries;)\n> \t\t\tif (pack_id == e->pack_id)\n> \t\t\t\t*c++ = &e->idx;\n> +\t\t\telse\n> +\t\t\t\tgoto done;\n> +done:\n> \tlast = idx + object_count;\n> \tif (c != last)\n> \t\tdie(\"internal consistency error creating the index\");\n> \n> \ttmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts, pack_data->sha1);\n> \n> But that seems to break some of the tests, so I think there is some case\n> which does not match my theory. :)\n\nYes, it's probably because duplicate objects from the object\nstore will create entries with e->pack_id == MAX_PACK_ID\n\n> Another strategy is to note that we cross-check against object_count\n> here. So you could simply break out of the loop when we have found\n> object_count matches.\n\nI'm worried that could fail to detect bugs which should've led\nus to die of the internal consistency error.\n\n\nWhat we could probably do more safely is limit the scan to only\nrecent object pools (new ones since the previous create_index call\nand the last one scanned).\n\nThis passes tests, but I could be missing something:\n\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -926,14 +926,16 @@ static const char *create_index(void)\n \tstruct pack_idx_entry **idx, **c, **last;\n \tstruct object_entry *e;\n \tstruct object_entry_pool *o;\n+\tstatic struct object_entry_pool *last_seen;\n \n \t/* Build the table of object IDs. */\n \tALLOC_ARRAY(idx, object_count);\n \tc = idx;\n-\tfor (o = blocks; o; o = o->next_pool)\n+\tfor (o = blocks; o; o = (o == last_seen) ? NULL : o->next_pool)\n \t\tfor (e = o->next_free; e-- != o->entries;)\n \t\t\tif (pack_id == e->pack_id)\n \t\t\t\t*c++ = &e->idx;\n+\tlast_seen = blocks;\n \tlast = idx + object_count;\n \tif (c != last)\n \t\tdie(\"internal consistency error creating the index\");\n"}]}