Re: [PATCH v2] commit.c: use the generic sha1_pos function for lookup

2014-02-27 Thread Junio C Hamano
Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru writes:

 Refactor binary search in commit_graft_pos function: use
 generic sha1_pos function.

 Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
 ---

Looks trivially correct; thanks.

Looking at this patch makes me wonder why we have sha1_pos() and
sha1_entry_pos() helper functions, though.  It feels as if the
former could be written in terms of the latter, but there may be
some performance and correctness downsides if we did so:

 - rewriting sha1_entry_pos() in terms of sha1_pos() would add the
   cost of callback to obtain the keys;

 - sha1_entry_pos() picks the middle location conservatively to
   avoid overshooting penalty, which sha1_pos() does not do;

 - sha1_entry_pos() has been updated recently to tolerate
   duplicates.



  commit.c | 24 +---
  1 file changed, 9 insertions(+), 15 deletions(-)

 diff --git a/commit.c b/commit.c
 index 6bf4fe0..6ceee6a 100644
 --- a/commit.c
 +++ b/commit.c
 @@ -10,6 +10,7 @@
  #include mergesort.h
  #include commit-slab.h
  #include prio-queue.h
 +#include sha1-lookup.h
  
  static struct commit_extra_header *read_commit_extra_header_lines(const char 
 *buf, size_t len, const char **);
  
 @@ -114,23 +115,16 @@ static unsigned long parse_commit_date(const char *buf, 
 const char *tail)
  static struct commit_graft **commit_graft;
  static int commit_graft_alloc, commit_graft_nr;
  
 +static const unsigned char *commit_graft_sha1_access(size_t index, void 
 *table)
 +{
 + struct commit_graft **commit_graft_table = table;
 + return commit_graft_table[index]-sha1;
 +}
 +
  static int commit_graft_pos(const unsigned char *sha1)
  {
 - int lo, hi;
 - lo = 0;
 - hi = commit_graft_nr;
 - while (lo  hi) {
 - int mi = (lo + hi) / 2;
 - struct commit_graft *graft = commit_graft[mi];
 - int cmp = hashcmp(sha1, graft-sha1);
 - if (!cmp)
 - return mi;
 - if (cmp  0)
 - hi = mi;
 - else
 - lo = mi + 1;
 - }
 - return -lo - 1;
 + return sha1_pos(sha1, commit_graft, commit_graft_nr,
 + commit_graft_sha1_access);
  }
  
  int register_commit_graft(struct commit_graft *graft, int ignore_dups)
--
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] commit.c: use the generic sha1_pos function for lookup

2014-02-26 Thread Dmitry S. Dolzhenko
Refactor binary search in commit_graft_pos function: use
generic sha1_pos function.

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 commit.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..6ceee6a 100644
--- a/commit.c
+++ b/commit.c
@@ -10,6 +10,7 @@
 #include mergesort.h
 #include commit-slab.h
 #include prio-queue.h
+#include sha1-lookup.h
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char 
*buf, size_t len, const char **);
 
@@ -114,23 +115,16 @@ static unsigned long parse_commit_date(const char *buf, 
const char *tail)
 static struct commit_graft **commit_graft;
 static int commit_graft_alloc, commit_graft_nr;
 
+static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
+{
+   struct commit_graft **commit_graft_table = table;
+   return commit_graft_table[index]-sha1;
+}
+
 static int commit_graft_pos(const unsigned char *sha1)
 {
-   int lo, hi;
-   lo = 0;
-   hi = commit_graft_nr;
-   while (lo  hi) {
-   int mi = (lo + hi) / 2;
-   struct commit_graft *graft = commit_graft[mi];
-   int cmp = hashcmp(sha1, graft-sha1);
-   if (!cmp)
-   return mi;
-   if (cmp  0)
-   hi = mi;
-   else
-   lo = mi + 1;
-   }
-   return -lo - 1;
+   return sha1_pos(sha1, commit_graft, commit_graft_nr,
+   commit_graft_sha1_access);
 }
 
 int register_commit_graft(struct commit_graft *graft, int ignore_dups)
-- 
1.8.3.2


--
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