Re: [PATCH v2 5/5] get_sha1: drop object/refname ambiguity flag

2014-01-08 Thread Michael Haggerty
On 01/08/2014 01:00 AM, Jeff King wrote:
 Now that our object/refname ambiguity test is much faster
 (thanks to the previous commit), there is no reason for code
 like cat-file --batch-check to turn it off. Here are
 before and after timings with this patch (on git.git):
 
   $ git rev-list --objects --all | cut -d' ' -f1 objects
 
   [with flag]
   $ best-of-five -i objects ./git cat-file --batch-check
   real0m0.392s
   user0m0.368s
   sys 0m0.024s
 
   [without flag, without speedup; i.e., pre-25fba78]
   $ best-of-five -i objects ./git cat-file --batch-check
   real0m1.652s
   user0m0.904s
   sys 0m0.748s
 
   [without flag, with speedup]
   $ best-of-five -i objects ./git cat-file --batch-check
   real0m0.388s
   user0m0.356s
   sys 0m0.028s
 
 So the new implementation does just as well as we did with
 the flag turning the whole thing off (better actually, but
 that is within the noise).
 
 Signed-off-by: Jeff King p...@peff.net
 [...]

Very nice.  Correctness without a performance hit.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 5/5] get_sha1: drop object/refname ambiguity flag

2014-01-07 Thread Jeff King
Now that our object/refname ambiguity test is much faster
(thanks to the previous commit), there is no reason for code
like cat-file --batch-check to turn it off. Here are
before and after timings with this patch (on git.git):

  $ git rev-list --objects --all | cut -d' ' -f1 objects

  [with flag]
  $ best-of-five -i objects ./git cat-file --batch-check
  real0m0.392s
  user0m0.368s
  sys 0m0.024s

  [without flag, without speedup; i.e., pre-25fba78]
  $ best-of-five -i objects ./git cat-file --batch-check
  real0m1.652s
  user0m0.904s
  sys 0m0.748s

  [without flag, with speedup]
  $ best-of-five -i objects ./git cat-file --batch-check
  real0m0.388s
  user0m0.356s
  sys 0m0.028s

So the new implementation does just as well as we did with
the flag turning the whole thing off (better actually, but
that is within the noise).

Signed-off-by: Jeff King p...@peff.net
---
 builtin/cat-file.c | 9 -
 cache.h| 1 -
 environment.c  | 1 -
 sha1_name.c| 2 +-
 4 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index ce79103..afba21f 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -285,15 +285,6 @@ static int batch_objects(struct batch_options *opt)
if (opt-print_contents)
data.info.typep = data.type;
 
-   /*
-* We are going to call get_sha1 on a potentially very large number of
-* objects. In most large cases, these will be actual object sha1s. The
-* cost to double-check that each one is not also a ref (just so we can
-* warn) ends up dwarfing the actual cost of the object lookups
-* themselves. We can work around it by just turning off the warning.
-*/
-   warn_on_object_refname_ambiguity = 0;
-
while (strbuf_getline(buf, stdin, '\n') != EOF) {
if (data.split_on_whitespace) {
/*
diff --git a/cache.h b/cache.h
index ce377e1..73afc38 100644
--- a/cache.h
+++ b/cache.h
@@ -566,7 +566,6 @@ extern int assume_unchanged;
 extern int prefer_symlink_refs;
 extern int log_all_ref_updates;
 extern int warn_ambiguous_refs;
-extern int warn_on_object_refname_ambiguity;
 extern int shared_repository;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
diff --git a/environment.c b/environment.c
index 3c76905..c59f6d4 100644
--- a/environment.c
+++ b/environment.c
@@ -22,7 +22,6 @@ int prefer_symlink_refs;
 int is_bare_repository_cfg = -1; /* unspecified */
 int log_all_ref_updates = -1; /* unspecified */
 int warn_ambiguous_refs = 1;
-int warn_on_object_refname_ambiguity = 1;
 int repository_format_version;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
diff --git a/sha1_name.c b/sha1_name.c
index f83ecb7..b9aaf74 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -451,7 +451,7 @@ static int get_sha1_basic(const char *str, int len, 
unsigned char *sha1)
int at, reflog_len, nth_prior = 0;
 
if (len == 40  !get_sha1_hex(str, sha1)) {
-   if (warn_ambiguous_refs  warn_on_object_refname_ambiguity) {
+   if (warn_ambiguous_refs) {
if (sha1_is_ambiguous_with_ref(sha1)) {
warning(warn_msg, len, str);
if (advice_object_name_warning)
-- 
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html