{"thread":{"id":"29097","subject":"[PATCH 0/5] cache-tree revisited","startedAt":"2011-12-06T17:43:34Z","lastAt":"2011-12-08T14:15:29Z","messageCount":10,"participants":["Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"180382","messageId":"cover.1323191497.git.trast@student.ethz.ch","threadId":"29097","inReplyTo":null,"subject":"[PATCH 0/5] cache-tree revisited","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-06T17:43:34Z","receivedAt":"2011-12-06T17:43:34Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Ahh, I forgot all about that exchange.\n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/178480/focus=178515\n> \n> The cache-tree mechanism has traditionally been one of the more important\n> optimizations and it would be very nice if we can resurrect the behaviour\n> for \"git commit\" too.\n\nOh, I buried that.  Let's try something other than the aggressive\nstrategy I had there: only compute cache-tree if\n\n* we know we're going to need it soon, and we're about to write out\n  the index anyway (as in git-commit)\n\n* we know we can rebuild it from a tree, and we're about to write out\n  the index anyway (as in git-reset)\n\nBoth of these are essentially free, so I'm not sure I should even\nbother with timings, but I ran the silly script at the end on my\nfavourite linux v2.6.37-rc2 repo, and it takes\n\n  before-:  real 0m29.103s   user 0m17.335s   sys 0m8.415s\n  before+:  real 0m30.860s   user 0m9.741s    sys 0m7.287s\n  after-:   real 0m28.798s   user 0m17.367s   sys 0m8.435s\n  after+:   real 0m17.563s   user 0m8.246s    sys 0m7.122s\n\nwhere the + signifies starting out with fully valid cache-tree data,\nand - means starting out with none at all.\n\nSo it's really free when it's starting out in bad shape, and it's\nmuch faster when it starts out with valid data.\n\nIf you insist on writing cache-tree at *every* (except --only) commit,\nthen we might make it so that it writes out the index again at the\nvery end if it didn't already update it earlier.  It would (and does,\nI've tried... ok ok, patch is at the end) of course give the +\nperformance in the - case, but it's not free so other operations may\nbe affected.\n\n\n\n---- 8< ---- test script for the above timings ---- 8< ----\n#!/bin/sh\n\nfor i in $(seq 1 100); do\n    echo $i > arch/alpha/boot/foo\n    git add arch/alpha/boot/foo\n    git commit -m$i\ndone\n---- >8 ----\n\n---- 8< ---- patch to let git-commit always write the index ---- 8< ----\ndiff --git i/builtin/commit.c w/builtin/commit.c\nindex 57d028e..8e0c773 100644\n--- i/builtin/commit.c\n+++ w/builtin/commit.c\n@@ -333,6 +333,8 @@ static void refresh_cache_or_die(int refresh_flags)\n \t\tdie_resolve_conflict(\"commit\");\n }\n \n+static int wrote_index_already = 0;\n+\n static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\t\t   const struct commit *current_head, int is_status)\n {\n@@ -395,6 +397,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tupdate_main_cache_tree(1);\n+\t\twrote_index_already = 1;\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(_(\"unable to write new_index file\"));\n@@ -415,6 +418,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tif (active_cache_changed) {\n+\t\t\twrote_index_already = 1;\n \t\t\tupdate_main_cache_tree(1);\n \t\t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t\t    commit_locked_index(&index_lock))\n@@ -639,6 +643,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tconst char *hook_arg2 = NULL;\n \tint ident_shown = 0;\n \tint clean_message_contents = (cleanup_mode != CLEANUP_NONE);\n+\tint fd;\n \n \tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n \t\treturn 0;\n@@ -863,11 +868,17 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t * the editor and after we invoke run_status above.\n \t */\n \tdiscard_cache();\n+\tif (!wrote_index_already)\n+\t\tfd = hold_locked_index(&index_lock, 1);\n \tread_cache_from(index_file);\n \tif (update_main_cache_tree(0)) {\n \t\terror(_(\"Error building trees\"));\n \t\treturn 0;\n \t}\n+\tif (!wrote_index_already && commit_style != COMMIT_PARTIAL)\n+\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t    close_lock_file(&index_lock))\n+\t\t\tdie(_(\"unable to write new_index file\"));\n \n \tif (run_hook(index_file, \"prepare-commit-msg\",\n \t\t     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))\n---- >8 ----\n\n\nThomas Rast (5):\n  Add test-scrap-cache-tree\n  Test the current state of the cache-tree optimization\n  Refactor cache_tree_update idiom from commit\n  commit: write cache-tree data when writing index anyway\n  reset: update cache-tree data when appropriate\n\n .gitignore              |    1 +\n Makefile                |    1 +\n builtin/commit.c        |    7 +--\n builtin/reset.c         |    7 +++\n cache-tree.c            |   19 +++++++--\n cache-tree.h            |    4 +-\n merge-recursive.c       |    2 +-\n t/t0090-cache-tree.sh   |   95 +++++++++++++++++++++++++++++++++++++++++++++++\n test-dump-cache-tree.c  |    2 +-\n test-scrap-cache-tree.c |   17 ++++++++\n 10 files changed, 144 insertions(+), 11 deletions(-)\n create mode 100755 t/t0090-cache-tree.sh\n create mode 100644 test-scrap-cache-tree.c\n\n-- \n1.7.8.425.ga639d3\n"},{"id":"180383","messageId":"534454506f92428863de8fe0638f4129e962a073.1323191497.git.trast@student.ethz.ch","threadId":"29097","inReplyTo":"cover.1323191497.git.trast@student.ethz.ch","subject":"[PATCH 1/5] Add test-scrap-cache-tree","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-06T17:43:35Z","receivedAt":"2011-12-06T17:43:35Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"A simple utility that invalidates all existing cache-tree data.  We\nneed this for tests.  (We don't need a tool to rebuild the cache-tree\ndata; git read-tree HEAD works for that.)\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n .gitignore              |    1 +\n Makefile                |    1 +\n test-scrap-cache-tree.c |   17 +++++++++++++++++\n 3 files changed, 19 insertions(+), 0 deletions(-)\n create mode 100644 test-scrap-cache-tree.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 8572c8c..122336c 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -171,6 +171,7 @@\n /test-date\n /test-delta\n /test-dump-cache-tree\n+/test-scrap-cache-tree\n /test-genrandom\n /test-index-version\n /test-line-buffer\ndiff --git a/Makefile b/Makefile\nindex cbb5601..6bfa10b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -436,6 +436,7 @@ TEST_PROGRAMS_NEED_X += test-ctype\n TEST_PROGRAMS_NEED_X += test-date\n TEST_PROGRAMS_NEED_X += test-delta\n TEST_PROGRAMS_NEED_X += test-dump-cache-tree\n+TEST_PROGRAMS_NEED_X += test-scrap-cache-tree\n TEST_PROGRAMS_NEED_X += test-genrandom\n TEST_PROGRAMS_NEED_X += test-index-version\n TEST_PROGRAMS_NEED_X += test-line-buffer\ndiff --git a/test-scrap-cache-tree.c b/test-scrap-cache-tree.c\nnew file mode 100644\nindex 0000000..4728013\n--- /dev/null\n+++ b/test-scrap-cache-tree.c\n@@ -0,0 +1,17 @@\n+#include \"cache.h\"\n+#include \"tree.h\"\n+#include \"cache-tree.h\"\n+\n+static struct lock_file index_lock;\n+\n+int main(int ac, char **av)\n+{\n+\tint fd = hold_locked_index(&index_lock, 1);\n+\tif (read_cache() < 0)\n+\t\tdie(\"unable to read index file\");\n+\tactive_cache_tree = NULL;\n+\tif (write_cache(fd, active_cache, active_nr)\n+\t    || commit_lock_file(&index_lock))\n+\t\tdie(\"unable to write index file\");\n+\treturn 0;\n+}\n-- \n1.7.8.425.ga639d3\n"},{"id":"180385","messageId":"b6b5b93a136baaa59173324cac85847f49786336.1323191497.git.trast@student.ethz.ch","threadId":"29097","inReplyTo":"cover.1323191497.git.trast@student.ethz.ch","subject":"[PATCH 2/5] Test the current state of the cache-tree optimization","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-06T17:43:36Z","receivedAt":"2011-12-06T17:43:36Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The cache-tree optimization originally helped speed up write-tree\noperation.  However, many commands no longer properly maintain -- or\nuse an opportunity to cheaply generate -- the cache-tree data.  In\nparticular, this affects commit, checkout and reset.  The notable\nexamples that *do* write cache-tree data are read-tree and write-tree.\n\nThis sadly means most people no longer benefit from the optimization,\nas they would not normally use the plumbing commands.\n\nDocument the current state of affairs in a test file, in preparation\nfor improvements in the area.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n t/t0090-cache-tree.sh |   95 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 95 insertions(+), 0 deletions(-)\n create mode 100755 t/t0090-cache-tree.sh\n\ndiff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\nnew file mode 100755\nindex 0000000..3d0702a\n--- /dev/null\n+++ b/t/t0090-cache-tree.sh\n@@ -0,0 +1,95 @@\n+#!/bin/sh\n+\n+test_description=\"Test whether cache-tree is properly updated\n+\n+Tests whether various commands properly update and/or rewrite the\n+cache-tree extension.\n+\"\n+ . ./test-lib.sh\n+\n+cmp_cache_tree () {\n+\ttest-dump-cache-tree >actual &&\n+\tsed \"s/$_x40/SHA/\" <actual >filtered &&\n+\ttest_cmp \"$1\" filtered\n+}\n+\n+# We don't bother with actually checking the SHA1:\n+# test-dump-cache-tree already verifies that all existing data is\n+# correct.\n+test_shallow_cache_tree () {\n+\techo \"SHA \" \\\n+\t    \"($(git ls-files|wc -l) entries, 0 subtrees)\" >expect &&\n+\tcmp_cache_tree expect\n+}\n+\n+test_invalid_cache_tree () {\n+\techo \"invalid                                   (0 subtrees)\" >expect &&\n+\techo \"SHA #(ref) \" \\\n+\t    \"($(git ls-files|wc -l) entries, 0 subtrees)\" >>expect &&\n+\tcmp_cache_tree expect\n+}\n+\n+test_no_cache_tree () {\n+\t: >expect &&\n+\tcmp_cache_tree expect\n+}\n+\n+test_expect_failure 'initial commit has cache-tree' '\n+\ttest_commit foo &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_expect_success 'read-tree HEAD establishes cache-tree' '\n+\tgit read-tree HEAD &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_expect_success 'git-add invalidates cache-tree' '\n+\ttest_when_finished \"git reset --hard; git read-tree HEAD\" &&\n+\techo \"I changed this file\" > foo &&\n+\tgit add foo &&\n+\ttest_invalid_cache_tree\n+'\n+\n+test_expect_success 'update-index invalidates cache-tree' '\n+\ttest_when_finished \"git reset --hard; git read-tree HEAD\" &&\n+\techo \"I changed this file\" > foo &&\n+\tgit update-index --add foo &&\n+\ttest_invalid_cache_tree\n+'\n+\n+test_expect_success 'write-tree establishes cache-tree' '\n+\ttest-scrap-cache-tree &&\n+\tgit write-tree &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_expect_success 'test-scrap-cache-tree works' '\n+\tgit read-tree HEAD &&\n+\ttest-scrap-cache-tree &&\n+\ttest_no_cache_tree\n+'\n+\n+test_expect_failure 'second commit has cache-tree' '\n+\ttest_commit bar &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_expect_failure 'reset --hard gives cache-tree' '\n+\ttest-scrap-cache-tree &&\n+\tgit reset --hard &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_expect_failure 'reset --hard without index gives cache-tree' '\n+\trm -f .git/index &&\n+\tgit reset --hard &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_expect_failure 'checkout gives cache-tree' '\n+\tgit checkout HEAD^ &&\n+\ttest_shallow_cache_tree\n+'\n+\n+test_done\n-- \n1.7.8.425.ga639d3\n"},{"id":"180384","messageId":"dc6cfeae970f465492050f190b7b0f93f6658cc1.1323191497.git.trast@student.ethz.ch","threadId":"29097","inReplyTo":"cover.1323191497.git.trast@student.ethz.ch","subject":"[PATCH 3/5] Refactor cache_tree_update idiom from commit","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-06T17:43:37Z","receivedAt":"2011-12-06T17:43:37Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"We'll need to safely create or update the cache-tree data of the_index\nfrom other places.  While at it, give it an argument that lets us\nsilence the messages produced by unmerged entries (which prevent it\nfrom working).\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n builtin/commit.c       |    5 +----\n cache-tree.c           |   19 +++++++++++++++----\n cache-tree.h           |    4 +++-\n merge-recursive.c      |    2 +-\n test-dump-cache-tree.c |    2 +-\n 5 files changed, 21 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e36e9ad..0163086 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -862,10 +862,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t */\n \tdiscard_cache();\n \tread_cache_from(index_file);\n-\tif (!active_cache_tree)\n-\t\tactive_cache_tree = cache_tree();\n-\tif (cache_tree_update(active_cache_tree,\n-\t\t\t      active_cache, active_nr, 0, 0) < 0) {\n+\tif (update_main_cache_tree(0)) {\n \t\terror(_(\"Error building trees\"));\n \t\treturn 0;\n \t}\ndiff --git a/cache-tree.c b/cache-tree.c\nindex f755590..8de3959 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -150,7 +150,7 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)\n }\n \n static int verify_cache(struct cache_entry **cache,\n-\t\t\tint entries)\n+\t\t\tint entries, int silent)\n {\n \tint i, funny;\n \n@@ -159,6 +159,8 @@ static int verify_cache(struct cache_entry **cache,\n \tfor (i = 0; i < entries; i++) {\n \t\tstruct cache_entry *ce = cache[i];\n \t\tif (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {\n+\t\t\tif (silent)\n+\t\t\t\treturn -1;\n \t\t\tif (10 < ++funny) {\n \t\t\t\tfprintf(stderr, \"...\\n\");\n \t\t\t\tbreak;\n@@ -370,10 +372,11 @@ int cache_tree_update(struct cache_tree *it,\n \t\t      struct cache_entry **cache,\n \t\t      int entries,\n \t\t      int missing_ok,\n-\t\t      int dryrun)\n+\t\t      int dryrun,\n+\t\t      int silent)\n {\n \tint i;\n-\ti = verify_cache(cache, entries);\n+\ti = verify_cache(cache, entries, silent);\n \tif (i)\n \t\treturn i;\n \ti = update_one(it, cache, entries, \"\", 0, missing_ok, dryrun);\n@@ -573,7 +576,7 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)\n \n \t\tif (cache_tree_update(active_cache_tree,\n \t\t\t\t      active_cache, active_nr,\n-\t\t\t\t      missing_ok, 0) < 0)\n+\t\t\t\t      missing_ok, 0, 0) < 0)\n \t\t\treturn WRITE_TREE_UNMERGED_INDEX;\n \t\tif (0 <= newfd) {\n \t\t\tif (!write_cache(newfd, active_cache, active_nr) &&\n@@ -668,3 +671,11 @@ int cache_tree_matches_traversal(struct cache_tree *root,\n \t\treturn it->entry_count;\n \treturn 0;\n }\n+\n+int update_main_cache_tree (int silent)\n+{\n+\tif (!the_index.cache_tree)\n+\t\tthe_index.cache_tree = cache_tree();\n+\treturn cache_tree_update(the_index.cache_tree,\n+\t\t\t\t the_index.cache, the_index.cache_nr, 0, 0, silent);\n+}\ndiff --git a/cache-tree.h b/cache-tree.h\nindex 3df641f..0ec0b2a 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -29,7 +29,9 @@ struct cache_tree {\n struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);\n \n int cache_tree_fully_valid(struct cache_tree *);\n-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int);\n+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int);\n+\n+int update_main_cache_tree(int);\n \n /* bitmasks to write_cache_as_tree flags */\n #define WRITE_TREE_MISSING_OK 1\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 5a2db29..d83cd6c 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n \n \tif (!cache_tree_fully_valid(active_cache_tree) &&\n \t    cache_tree_update(active_cache_tree,\n-\t\t\t      active_cache, active_nr, 0, 0) < 0)\n+\t\t\t      active_cache, active_nr, 0, 0, 0) < 0)\n \t\tdie(\"error building trees\");\n \n \tresult = lookup_tree(active_cache_tree->sha1);\ndiff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c\nindex 1f73f1e..e6c2923 100644\n--- a/test-dump-cache-tree.c\n+++ b/test-dump-cache-tree.c\n@@ -59,6 +59,6 @@ int main(int ac, char **av)\n \tstruct cache_tree *another = cache_tree();\n \tif (read_cache() < 0)\n \t\tdie(\"unable to read index file\");\n-\tcache_tree_update(another, active_cache, active_nr, 0, 1);\n+\tcache_tree_update(another, active_cache, active_nr, 0, 1, 0);\n \treturn dump_cache_tree(active_cache_tree, another, \"\");\n }\n-- \n1.7.8.425.ga639d3\n"},{"id":"180387","messageId":"fcb51622a521eaf418c82bfd5ff83e8a0b79498b.1323191497.git.trast@student.ethz.ch","threadId":"29097","inReplyTo":"cover.1323191497.git.trast@student.ethz.ch","subject":"[PATCH 4/5] commit: write cache-tree data when writing index anyway","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-06T17:43:38Z","receivedAt":"2011-12-06T17:43:38Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"In prepare_index(), we refresh the index, and then write it to disk if\nthis changed the index data.  After running hooks we re-read the index\nand compute the root tree sha1 with the cache-tree machinery.\n\nThis gives us a mostly free opportunity to write up-to-date cache-tree\ndata: we can compute it in prepare_index() immediately before writing\nthe index to disk.\n\nIf we do this, we were going to write the index anyway, and the later\ncache-tree update has no further work to do.  If we don't do it, we\ndon't do any extra work, though we still don't have have cache-tree\ndata after the commit.\n\nThe only case that suffers badly is when the pre-commit hook changes\nmany trees in the index.  I'm writing this off as highly unusual.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n\nDon't ask me why git thinks the index has been changed only when\nalready building upon an earlier commit.  I don't know.  I suspect\nit's some raciness issue though.\n\n builtin/commit.c      |    2 ++\n t/t0090-cache-tree.sh |    2 +-\n 2 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 0163086..57d028e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -394,6 +394,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n \t\trefresh_cache_or_die(refresh_flags);\n+\t\tupdate_main_cache_tree(1);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(_(\"unable to write new_index file\"));\n@@ -414,6 +415,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tif (active_cache_changed) {\n+\t\t\tupdate_main_cache_tree(1);\n \t\t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t\t    commit_locked_index(&index_lock))\n \t\t\t\tdie(_(\"unable to write new_index file\"));\ndiff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\nindex 3d0702a..a3527a5 100755\n--- a/t/t0090-cache-tree.sh\n+++ b/t/t0090-cache-tree.sh\n@@ -70,7 +70,7 @@ test_expect_success 'test-scrap-cache-tree works' '\n \ttest_no_cache_tree\n '\n \n-test_expect_failure 'second commit has cache-tree' '\n+test_expect_success 'second commit has cache-tree' '\n \ttest_commit bar &&\n \ttest_shallow_cache_tree\n '\n-- \n1.7.8.425.ga639d3\n"},{"id":"180386","messageId":"1385c10084ae41ae4543ef3ccaa1d6c8182b2204.1323191497.git.trast@student.ethz.ch","threadId":"29097","inReplyTo":"cover.1323191497.git.trast@student.ethz.ch","subject":"[PATCH 5/5] reset: update cache-tree data when appropriate","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-06T17:43:39Z","receivedAt":"2011-12-06T17:43:39Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"In the case of --mixed and --hard, we throw away the old index and\nrebuild everything from the tree argument (or HEAD).  So we have an\nopportunity here to fill in the cache-tree data, just as read-tree\ndid.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n builtin/reset.c       |    7 +++++++\n t/t0090-cache-tree.sh |    4 ++--\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 811e8e2..8c2c1d5 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -43,6 +43,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \tint nr = 1;\n \tint newfd;\n \tstruct tree_desc desc[2];\n+\tstruct tree *tree;\n \tstruct unpack_trees_options opts;\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \n@@ -84,6 +85,12 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \t\treturn error(_(\"Failed to find tree of %s.\"), sha1_to_hex(sha1));\n \tif (unpack_trees(nr, desc, &opts))\n \t\treturn -1;\n+\n+\tif (reset_type == MIXED || reset_type == HARD) {\n+\t\ttree = parse_tree_indirect(sha1);\n+\t\tprime_cache_tree(&active_cache_tree, tree);\n+\t}\n+\n \tif (write_cache(newfd, active_cache, active_nr) ||\n \t    commit_locked_index(lock))\n \t\treturn error(_(\"Could not write new index file.\"));\ndiff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh\nindex a3527a5..f972562 100755\n--- a/t/t0090-cache-tree.sh\n+++ b/t/t0090-cache-tree.sh\n@@ -75,13 +75,13 @@ test_expect_success 'second commit has cache-tree' '\n \ttest_shallow_cache_tree\n '\n \n-test_expect_failure 'reset --hard gives cache-tree' '\n+test_expect_success 'reset --hard gives cache-tree' '\n \ttest-scrap-cache-tree &&\n \tgit reset --hard &&\n \ttest_shallow_cache_tree\n '\n \n-test_expect_failure 'reset --hard without index gives cache-tree' '\n+test_expect_success 'reset --hard without index gives cache-tree' '\n \trm -f .git/index &&\n \tgit reset --hard &&\n \ttest_shallow_cache_tree\n-- \n1.7.8.425.ga639d3\n"},{"id":"180426","messageId":"7vfwgxe2k6.fsf@alter.siamese.dyndns.org","threadId":"29097","inReplyTo":"534454506f92428863de8fe0638f4129e962a073.1323191497.git.trast@student.ethz.ch","subject":"Re: [PATCH 1/5] Add test-scrap-cache-tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-06T22:51:53Z","receivedAt":"2011-12-06T22:51:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@student.ethz.ch> writes:\n\n> A simple utility that invalidates all existing cache-tree data.  We\n> need this for tests.  (We don't need a tool to rebuild the cache-tree\n> data; git read-tree HEAD works for that.)\n\n\"git read-tree -m HEAD HEAD\" would work for that, I suspect ;-)\n\nBut that can be improved later, so it is very sensible to add a test tool\nlike this.\n"},{"id":"180429","messageId":"7v62hte1k7.fsf@alter.siamese.dyndns.org","threadId":"29097","inReplyTo":"1385c10084ae41ae4543ef3ccaa1d6c8182b2204.1323191497.git.trast@student.ethz.ch","subject":"Re: [PATCH 5/5] reset: update cache-tree data when appropriate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-06T23:13:28Z","receivedAt":"2011-12-06T23:13:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@student.ethz.ch> writes:\n\n> In the case of --mixed and --hard, we throw away the old index and\n> rebuild everything from the tree argument (or HEAD).  So we have an\n> opportunity here to fill in the cache-tree data, just as read-tree\n> did.\n>\n> Signed-off-by: Thomas Rast <trast@student.ethz.ch>\n> ---\n>  builtin/reset.c       |    7 +++++++\n>  t/t0090-cache-tree.sh |    4 ++--\n>  2 files changed, 9 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 811e8e2..8c2c1d5 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -43,6 +43,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n>  \tint nr = 1;\n>  \tint newfd;\n>  \tstruct tree_desc desc[2];\n> +\tstruct tree *tree;\n>  \tstruct unpack_trees_options opts;\n>  \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n>  \n> @@ -84,6 +85,12 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n>  \t\treturn error(_(\"Failed to find tree of %s.\"), sha1_to_hex(sha1));\n>  \tif (unpack_trees(nr, desc, &opts))\n>  \t\treturn -1;\n> +\n> +\tif (reset_type == MIXED || reset_type == HARD) {\n> +\t\ttree = parse_tree_indirect(sha1);\n> +\t\tprime_cache_tree(&active_cache_tree, tree);\n> +\t}\n\nThe basic idea that MIXED or HARD should result in a cache-tree that match\nthe tree we just read is sound, but how expensive is prime_cache_tree()? I\nthink it reads the same tree once again. Admittedly, the data needed to\nreconstruct the tree is likely to be hot in core, but it may be necessary\nto measure before deciding if this is a good change.\n"},{"id":"180462","messageId":"201112070853.15364.trast@student.ethz.ch","threadId":"29097","inReplyTo":"7v62hte1k7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] reset: update cache-tree data when appropriate","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-07T07:53:15Z","receivedAt":"2011-12-07T07:53:15Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Thomas Rast <trast@student.ethz.ch> writes:\n> > @@ -84,6 +85,12 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n> >  \t\treturn error(_(\"Failed to find tree of %s.\"), sha1_to_hex(sha1));\n> >  \tif (unpack_trees(nr, desc, &opts))\n> >  \t\treturn -1;\n> > +\n> > +\tif (reset_type == MIXED || reset_type == HARD) {\n> > +\t\ttree = parse_tree_indirect(sha1);\n> > +\t\tprime_cache_tree(&active_cache_tree, tree);\n> > +\t}\n> \n> The basic idea that MIXED or HARD should result in a cache-tree that match\n> the tree we just read is sound, but how expensive is prime_cache_tree()? I\n> think it reads the same tree once again. Admittedly, the data needed to\n> reconstruct the tree is likely to be hot in core, but it may be necessary\n> to measure before deciding if this is a good change.\n\nOh, I just didn't bother with timings because it's the same strategy\nthat read-tree follows, so I assumed if it was worth it back then, it\nshould again be a win.\n\nFor linux-2.6 as usual and best-of-10:\n\n  git reset            before: 0:00.25real 0.12user 0.11system\n\t               after:  0:00.28real 0.14user 0.12system\n\n  git reset --hard     before: 0:00.21real 0.13user 0.06system\n                       after:  0:00.18real 0.10user 0.07system\n\nSo we spend ~30ms of extra user time, at a possible gain of 30% in\nfuture git-commit invocations, as mentioned in the cover letter.  I\nthink that's worth it :-)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"180589","messageId":"201112081515.29652.trast@student.ethz.ch","threadId":"29097","inReplyTo":"cover.1323191497.git.trast@student.ethz.ch","subject":"Re: [PATCH 0/5] cache-tree revisited","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-08T14:15:29Z","receivedAt":"2011-12-08T14:15:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> Junio C Hamano wrote:\n> > Ahh, I forgot all about that exchange.\n> > \n> >   http://thread.gmane.org/gmane.comp.version-control.git/178480/focus=178515\n> > \n> > The cache-tree mechanism has traditionally been one of the more important\n> > optimizations and it would be very nice if we can resurrect the behaviour\n> > for \"git commit\" too.\n> \n> Oh, I buried that.  Let's try something other than the aggressive\n> strategy I had there: only compute cache-tree if\n> \n> * we know we're going to need it soon, and we're about to write out\n>   the index anyway (as in git-commit)\n\nI had another idea: we could write out *just* a new cache-tree data\nset at the end of git-commit.\n\nDoing it the cheap way would mean rehashing the on-disk data without\nactually touching it.  (That might not be so bad, but then if your\nindex is small, why is writing it from scratch expensive?)\n\nDoing it efficiently requires making the sha1 restartable, which is\nentirely doable withblock-sha1/sha1.h (I haven't looked into\nppc/sha1.h).  As far as I can see it's not feasible with openssl's\nsha1.\n\nThat is, we would add a new index extension (say PSHA: partial SHA)\nand structure the index as\n\n  signature\n  header\n  cache data\n  PSHA <sha state up until just before PSHA>\n  TREE ...\n  [REUC ...]\n  sha1 footer\n\nThen it's easy to cheaply replace only the extensions, by restarting\nthe hashing from the PSHA data and re-emitting only the extension\ndata.\n\nI think all the bits are in place, and it would be easy to do.\nHowever, for it to make sense, we would have to make BLK_SHA1 the\ndefault for the most-used platforms and also not mind extending the\nSHA1 API.  Do you think that would fly?\n\nI thought about other ways to make the index writing restartable from\nthe middle, but the only clean approach I came up with would require a\nformat change to something like\n\n     signature\n  0  header\n  1  cache data\n  2  sha1 of 0..1\n  3  extension data A\n  4  sha1 of 2..3\n  5  extension data B\n  6  sha1 of 4..5\n  [possibly more]\n  7  end-of-index marker\n  8  sha1 of 6..7\n\netc.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"}]}