git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 9/9] clone: run check_everything_connected

From
Duy Nguyen <pclouds@gmail.com>
Date
Mar 31, 2013, 07:57 UTC
Message-ID
<20130331075712.GA13136@lanh>
In-Reply-To
<CACsJy8BMfYnFv=PL8x5JOMkjYc39h630oNEdukkjmBKBTNCibg@mail.gmail.com>
On Thu, Mar 28, 2013 at 07:40:51AM +0700, Duy Nguyen wrote:
Show 13 quoted lines
> Maybe we could do it in index-pack to save some (wall) time. I haven't
> tried but I think it might work. The problem is to make sure the pack
> contains objects for all sha1 references in the pack. By that
> description, we don't need to do standard DAG traversal. We could
> extract sha-1 references in index-pack as we uncompress objects and
> put all "want" sha-1 in a hash table. At the end of index-pack, we
> check if any sha-1 in the hash table still points to non-existing
> object.
> 
> This way, at least we don't need to uncompress all objects again in
> rev-list. We could parse+hash in both phases in index-pack. The first
> phase (parse_pack_objects) is usually I/O bound, we could hide some
> cost there. The second phase is multithreaded, all the better.

It looks like what I describe above is exactly what index-pack --strict does. Except that it holds the lock longer and has more abstraction layers to slow things down. On linux-2.6 with 3 threads:

$ rev-list --all --objects --quiet (aka check_everything_connected) 34.26user 0.22system 0:34.56elapsed 99%CPU (0avgtext+0avgdata 2550528maxresident)k 0inputs+0outputs (0major+208569minor)pagefaults 0swaps

$ index-pack --stdin 214.57user 8.38system 1:31.82elapsed 242%CPU (0avgtext+0avgdata 1357328maxresident)k 8inputs+1421016outputs (0major+1222537minor)pagefaults 0swaps

$ index-pack --stdin --strict 297.36user 13.77system 2:11.82elapsed 236%CPU (0avgtext+0avgdata 1875040maxresident)k 0inputs+1421016outputs (0major+1308718minor)pagefaults 0swaps

$ index-pack --stdin --connectivity 231.09user 7.42system 1:37.39elapsed 244%CPU (0avgtext+0avgdata 2080816maxresident)k 0inputs+1421016outputs (0major+540069minor)pagefaults 0swaps

The last one does not hold locks by duplicating object hash table per thread. As you can see the consumed memory is much higher than --stdin. In return it only adds up 1/3 of rev-list time.

Maybe you should check which one is cheaper for clone case, check_everything_connected() or index-pack --strict. -- Duy

Previous: Duy Nguyen
Message 60 of 60 in “propagating repo corruption across clone”
  1. Jeff KingMar 24, 2013
  2. Ævar Arnfjörð BjarmasonMar 24, 2013
  3. Jeff KingMar 24, 2013
  4. Jeff MitchellMar 25, 2013
  5. Jeff KingMar 25, 2013
  6. Duy NguyenMar 25, 2013
  7. Jeff KingMar 25, 2013
  8. Jeff MitchellMar 25, 2013
  9. Jeff KingMar 25, 2013
  10. Jeff MitchellMar 26, 2013
  11. Jeff KingMar 26, 2013
  12. Philip OakleyMar 26, 2013
  13. Jeff KingMar 26, 2013
  14. Rich FrommMar 26, 2013
  15. Jonathan NiederMar 27, 2013
  16. Rich FrommMar 27, 2013
  17. Jeff KingMar 27, 2013
  18. Jeff KingMar 27, 2013
  19. Junio C HamanoMar 27, 2013
  20. Sitaram ChamartyMar 27, 2013
  21. Junio C HamanoMar 27, 2013
  22. Sitaram ChamartyMar 27, 2013
  23. Rich FrommMar 27, 2013
  24. Junio C HamanoMar 27, 2013
  25. Jeff MitchellMar 28, 2013
  26. Jeff MitchellMar 28, 2013
  27. Duy NguyenMar 26, 2013
  28. Ilari LiusvaaraMar 24, 2013
  29. Junio C HamanoMar 25, 2013
  30. Jeff KingMar 25, 2013
  31. 0/9 corrupt object potpourriJeff King, Mar 25, 2013
  32. 1/9 stream_blob_to_fd: detect errors reading from streamJeff King, Mar 25, 2013
  33. Junio C HamanoMar 26, 2013
  34. 2/9 check_sha1_signature: check return value from read_istreamJeff King, Mar 25, 2013
  35. 3/9 read_istream_filtered: propagate read error from upstreamJeff King, Mar 25, 2013
  36. 4/9 avoid infinite loop in read_istream_looseJeff King, Mar 25, 2013
  37. 5/9 add test for streaming corrupt blobsJeff King, Mar 25, 2013
  38. Jonathan NiederMar 25, 2013
  39. Jeff KingMar 25, 2013
  40. Jeff KingMar 27, 2013
  41. Junio C HamanoMar 27, 2013
  42. 6/9 streaming_write_entry: propagate streaming errorsJeff King, Mar 25, 2013
  43. Eric SunshineMar 25, 2013
  44. Jeff KingMar 25, 2013
  45. Jonathan NiederMar 25, 2013
  46. 6/9 streaming_write_entry: propagate streaming errorsJeff King, Mar 25, 2013
  47. Jonathan NiederMar 25, 2013
  48. Junio C HamanoMar 26, 2013
  49. 7/9 add tests for cloning corrupted repositoriesJeff King, Mar 25, 2013
  50. 8/9 clone: die on errors from unpack_treesJeff King, Mar 25, 2013
  51. Junio C HamanoMar 26, 2013
  52. 10/9 clone: leave repo in place after checkout errorsJeff King, Mar 26, 2013
  53. Jonathan NiederMar 26, 2013
  54. Jeff KingMar 27, 2013
  55. 9/9 clone: run check_everything_connectedJeff King, Mar 25, 2013
  56. Duy NguyenMar 26, 2013
  57. Jeff KingMar 26, 2013
  58. Junio C HamanoMar 26, 2013
  59. Duy NguyenMar 28, 2013
  60. Duy NguyenMar 31, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.