{"thread":{"id":"32418","subject":"[PATCH] oneway_merge(): only lstat() when told to update worktree","startedAt":"2012-12-20T17:37:47Z","lastAt":"2012-12-20T21:03:36Z","messageCount":3,"participants":["Martin von Zweigbergk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"205273","messageId":"1356025067-5396-1-git-send-email-martinvonz@gmail.com","threadId":"32418","inReplyTo":null,"subject":"[PATCH] oneway_merge(): only lstat() when told to update worktree","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-12-20T17:37:47Z","receivedAt":"2012-12-20T17:37:47Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Although the subject line of 613f027 (read-tree -u one-way merge fix\nto check out locally modified paths., 2006-05-15) mentions \"read-tree\n-u\", it did not seem to check whether -u was in effect. Not checking\nwhether -u is in effect makes e.g. \"read-tree --reset\" lstat() the\nworktree, even though the worktree stat should not matter for that\noperation.\n\nThis speeds up e.g. \"git reset\" a little on the linux-2.6 repo (best\nof five, warm cache):\n\n        Before      After\nreal    0m0.288s    0m0.233s\nuser    0m0.190s    0m0.150s\nsys     0m0.090s    0m0.080s\n---\n\nI am very unfamiliar with this part of git, so my attempt at a\nmotivation may be totally off.\n\nI have another twenty-or-so patches to reset.c coming up that take the\ntimings down to around 90 ms, but this patch was quite unrelated to\nthat. Those other patches actually make this patch pointless for \"git\nreset\" (it takes another path), but I hope this is still a good change\nfor other operations that use oneway_merge.\n\n unpack-trees.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 6d96366..61acc5e 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1834,7 +1834,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n \n \tif (old && same(old, a)) {\n \t\tint update = 0;\n-\t\tif (o->reset && !ce_uptodate(old) && !ce_skip_worktree(old)) {\n+\t\tif (o->reset && o->update && !ce_uptodate(old) && !ce_skip_worktree(old)) {\n \t\t\tstruct stat st;\n \t\t\tif (lstat(old->name, &st) ||\n \t\t\t    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE))\n-- \n1.8.0.1.240.ge8a1f5a\n"},{"id":"205289","messageId":"7vk3sc4hle.fsf@alter.siamese.dyndns.org","threadId":"32418","inReplyTo":"1356025067-5396-1-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH] oneway_merge(): only lstat() when told to update worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-20T20:02:53Z","receivedAt":"2012-12-20T20:02:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martinvonz@gmail.com> writes:\n\n> Although the subject line of 613f027 (read-tree -u one-way merge fix\n> to check out locally modified paths., 2006-05-15) mentions \"read-tree\n> -u\", it did not seem to check whether -u was in effect. Not checking\n> whether -u is in effect makes e.g. \"read-tree --reset\" lstat() the\n> worktree, even though the worktree stat should not matter for that\n> operation.\n>\n> This speeds up e.g. \"git reset\" a little on the linux-2.6 repo (best\n> of five, warm cache):\n>\n>         Before      After\n> real    0m0.288s    0m0.233s\n> user    0m0.190s    0m0.150s\n> sys     0m0.090s    0m0.080s\n> ---\n\nSign-off?\n\nI briefly discussed this with Martin in person and came to the same\nconclusion. To me this looks like an obvious performance fix, but an\nindependent code audit catches our mistakes is of course welcomed.\n\nThanks.\n\n> I am very unfamiliar with this part of git, so my attempt at a\n> motivation may be totally off.\n>\n> I have another twenty-or-so patches to reset.c coming up that take the\n> timings down to around 90 ms, but this patch was quite unrelated to\n> that. Those other patches actually make this patch pointless for \"git\n> reset\" (it takes another path), but I hope this is still a good change\n> for other operations that use oneway_merge.\n>\n>  unpack-trees.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 6d96366..61acc5e 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -1834,7 +1834,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n>  \n>  \tif (old && same(old, a)) {\n>  \t\tint update = 0;\n> -\t\tif (o->reset && !ce_uptodate(old) && !ce_skip_worktree(old)) {\n> +\t\tif (o->reset && o->update && !ce_uptodate(old) && !ce_skip_worktree(old)) {\n>  \t\t\tstruct stat st;\n>  \t\t\tif (lstat(old->name, &st) ||\n>  \t\t\t    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE))\n"},{"id":"205294","messageId":"1356037416-23527-1-git-send-email-martinvonz@gmail.com","threadId":"32418","inReplyTo":"7vk3sc4hle.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] oneway_merge(): only lstat() when told to update worktree","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-12-20T21:03:36Z","receivedAt":"2012-12-20T21:03:36Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Although the subject line of 613f027 (read-tree -u one-way merge fix\nto check out locally modified paths., 2006-05-15) mentions \"read-tree\n-u\", it did not seem to check whether -u was in effect. Not checking\nwhether -u is in effect makes e.g. \"read-tree --reset\" lstat() the\nworktree, even though the worktree stat should not matter for that\noperation.\n\nThis speeds up e.g. \"git reset\" a little on the linux-2.6 repo (best\nof five, warm cache):\n\n        Before      After\nreal    0m0.288s    0m0.233s\nuser    0m0.190s    0m0.150s\nsys     0m0.090s    0m0.080s\n\nSigned-off-by: Martin von Zweigbergk <martinvonz@gmail.com>\n---\n unpack-trees.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 6d96366..61acc5e 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1834,7 +1834,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n \n \tif (old && same(old, a)) {\n \t\tint update = 0;\n-\t\tif (o->reset && !ce_uptodate(old) && !ce_skip_worktree(old)) {\n+\t\tif (o->reset && o->update && !ce_uptodate(old) && !ce_skip_worktree(old)) {\n \t\t\tstruct stat st;\n \t\t\tif (lstat(old->name, &st) ||\n \t\t\t    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE))\n-- \n1.8.0.1.240.ge8a1f5a\n"}]}