{"thread":{"id":"3102","subject":"[RFC/PATCH] Revert \"check_packed_git_idx(): check integrity of the idx file itself.\"","startedAt":"2006-01-19T04:34:22Z","lastAt":"2006-01-19T04:34:22Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"14868","messageId":"7vpsmokf8x.fsf@assigned-by-dhcp.cox.net","threadId":"3102","inReplyTo":null,"subject":"[RFC/PATCH] Revert \"check_packed_git_idx(): check integrity of the idx file itself.\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-01-19T04:34:22Z","receivedAt":"2006-01-19T04:34:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew noticed that this format-patch in the linux-2.6\nrepository runs very very very slowly.  I tried it myself and\nfound it to be 4x slowdown in a repository where I have both\ncommits still not packed:\n\n   $ git format-patch 7c5c220e 7c4d3367\n\n0.99.9m ran this with 20 seconds wallclock on a Duron 850, while\n1.1.3 (and the master) took 47 seconds wallclock.  Comparing\nuser time is worse: 10 seconds vs 37 seconds.\n\nIt turns out that this is due to the pack idx file integrity\ncheck I added about a month ago (bisect is cool -- thanks\nLinus):\n\n    commit c5ced64578a82b9d172aceb2f67c6fb9e639f6d9\n    Author: Junio C Hamano <junkio@cox.net>\n    Date:   Thu Dec 22 22:57:20 2005 -0800\n\n        check_packed_git_idx(): check integrity of the idx file itself.\n\n        Although pack-check.c had routine to verify the checksum for the\n        pack index file itself, the core did not check it before using\n        it.\n\nThe potential problem I saw with the core before this fix was\nthat after mapping the pack idx file and did some sanity\nchecks, we did not check the checksum we had in the idx file\nthat made sure the idx file itself is not corrupt.  Doing it\neverytime turned out to be quite expensive as illustrated by\nthis problem.  With the revert, the same test runs as fast as\n0.99.9m used to.\n\ngit-fsck-objects should catch it if idx file is corrupt, so this\nis removing suspender while we still have belt.\n\nSo I am going to propose reverting this \"safety measure\";\nobjections?\n\n-- >8 --\nDate: Wed Jan 18 20:26:14 2006 -0800\n\nThis reverts c5ced64578a82b9d172aceb2f67c6fb9e639f6d9 commit.\nIt turns out that doing this check every time we map the idx file\nis quite expensive.  A corrupt idx file is caught by git-fsck-objects,\nso this check is not strictly necessary.\n\nIn one unscientific test, 0.99.9m spent 10 seconds usertime for\nthe same task 1.1.3 takes 37 seconds usertime.  Reverting this gives\nus the performance of 0.99.9 back.\n\n---\n\n sha1_file.c |   16 +---------------\n 1 files changed, 1 insertions(+), 15 deletions(-)\n\na53f669407a76d01c7271124d06366a2dbd5a12d\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8bebbb2..20f6419 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -349,16 +349,12 @@ struct packed_git *packed_git;\n static int check_packed_git_idx(const char *path, unsigned long *idx_size_,\n \t\t\t\tvoid **idx_map_)\n {\n-\tSHA_CTX ctx;\n-\tunsigned char sha1[20];\n \tvoid *idx_map;\n \tunsigned int *index;\n \tunsigned long idx_size;\n \tint nr, i;\n-\tint fd;\n+\tint fd = open(path, O_RDONLY);\n \tstruct stat st;\n-\n-\tfd = open(path, O_RDONLY);\n \tif (fd < 0)\n \t\treturn -1;\n \tif (fstat(fd, &st)) {\n@@ -396,16 +392,6 @@ static int check_packed_git_idx(const ch\n \tif (idx_size != 4*256 + nr * 24 + 20 + 20)\n \t\treturn error(\"wrong index file size\");\n \n-\t/*\n-\t * File checksum.\n-\t */\n-\tSHA1_Init(&ctx);\n-\tSHA1_Update(&ctx, idx_map, idx_size-20);\n-\tSHA1_Final(sha1, &ctx);\n-\n-\tif (memcmp(sha1, idx_map + idx_size - 20, 20))\n-\t\treturn error(\"index checksum mismatch\");\n-\n \treturn 0;\n }\n \n-- \n1.1.3.gce7b\n"}]}