Re: [PATCH v4 0/22] pack bitmaps

2013-12-21 Thread Thomas Rast
Jeff King p...@peff.net writes:

 Here's the v4 re-roll of the pack bitmap series.

 The changes from v3 are:

  - reworked add_object_entry refactoring (see patch 11, which is new,
and patch 12 which builds on it in a more natural way)

This now looks like this (pasting because it is hard to see in the diffs):

  static int add_object_entry(const unsigned char *sha1, enum object_type type,
  const char *name, int exclude)
  {
  struct packed_git *found_pack;
  off_t found_offset;
  uint32_t index_pos;

  if (have_duplicate_entry(sha1, exclude, index_pos))
  return 0;

  if (!want_object_in_pack(sha1, exclude, found_pack, found_offset))
  return 0;

  create_object_entry(sha1, type, pack_name_hash(name),
  exclude, name  no_try_delta(name),
  index_pos, found_pack, found_offset);

  display_progress(progress_state, to_pack.nr_objects);
  return 1;
  }

  static int add_object_entry_from_bitmap(const unsigned char *sha1,
  enum object_type type,
  int flags, uint32_t name_hash,
  struct packed_git *pack, off_t offset)
  {
  uint32_t index_pos;

  if (have_duplicate_entry(sha1, 0, index_pos))
  return 0;

  create_object_entry(sha1, type, name_hash, 0, 0, index_pos, pack, 
offset);

  display_progress(progress_state, to_pack.nr_objects);
  return 1;
  }


Much nicer.  Thanks for going the extra mile!

-- 
Thomas Rast
t...@thomasrast.ch
--
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 v4 0/22] pack bitmaps

2013-12-21 Thread Jeff King
Here's the v4 re-roll of the pack bitmap series.

The changes from v3 are:

 - reworked add_object_entry refactoring (see patch 11, which is new,
   and patch 12 which builds on it in a more natural way)

 - better error/die reporting from write_reused_pack

 - added Ramsay's PRIx64 compat fix

 - fixed a user-after-free in the warning message of open_pack_bitmap_1

 - minor typo/thinko fixes from Thomas in docs and tests

Interdiff is below.

  [01/23]: sha1write: make buffer const-correct
  [02/23]: revindex: Export new APIs
  [03/23]: pack-objects: Refactor the packing list
  [04/23]: pack-objects: factor out name_hash
  [05/23]: revision: allow setting custom limiter function
  [06/23]: sha1_file: export `git_open_noatime`
  [07/23]: compat: add endianness helpers
  [08/23]: ewah: compressed bitmap implementation
  [09/23]: documentation: add documentation for the bitmap format
  [10/23]: pack-bitmap: add support for bitmap indexes
  [11/23]: pack-objects: split add_object_entry
  [12/23]: pack-objects: use bitmaps when packing objects
  [13/23]: rev-list: add bitmap mode to speed up object lists
  [14/23]: pack-objects: implement bitmap writing
  [15/23]: repack: stop using magic number for ARRAY_SIZE(exts)
  [16/23]: repack: turn exts array into array-of-struct
  [17/23]: repack: handle optional files created by pack-objects
  [18/23]: repack: consider bitmaps when performing repacks
  [19/23]: count-objects: recognize .bitmap in garbage-checking
  [20/23]: t: add basic bitmap functionality tests
  [21/23]: t/perf: add tests for pack bitmaps
  [22/23]: pack-bitmap: implement optional name_hash cache
  [23/23]: compat/mingw.h: Fix the MinGW and msvc builds

---
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e6d3922..499a3c4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1866,7 +1866,7 @@ pack.useBitmaps::
 
 pack.writebitmaps::
When true, git will write a bitmap index when packing all
-   objects to disk (e.g., as when `git repack -a` is run).  This
+   objects to disk (e.g., when `git repack -a` is run).  This
index can speed up the counting objects phase of subsequent
packs created for clones and fetches, at the cost of some disk
space and extra time spent on the initial repack.  Defaults to
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4504789..fd74197 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -72,11 +72,6 @@ static unsigned long cache_max_small_delta_size = 1000;
 
 static unsigned long window_memory_limit = 0;
 
-enum {
-   OBJECT_ENTRY_EXCLUDE = (1  0),
-   OBJECT_ENTRY_NO_TRY_DELTA = (1  1)
-};
-
 /*
  * stats
  */
@@ -712,21 +707,20 @@ static struct object_entry **compute_write_order(void)
 
 static off_t write_reused_pack(struct sha1file *f)
 {
-   uint8_t buffer[8192];
+   unsigned char buffer[8192];
off_t to_write;
int fd;
 
if (!is_pack_valid(reuse_packfile))
-   return 0;
+   die(packfile is invalid: %s, reuse_packfile-pack_name);
 
fd = git_open_noatime(reuse_packfile-pack_name);
if (fd  0)
-   return 0;
+   die_errno(unable to open packfile for reuse: %s,
+ reuse_packfile-pack_name);
 
-   if (lseek(fd, sizeof(struct pack_header), SEEK_SET) == -1) {
-   close(fd);
-   return 0;
-   }
+   if (lseek(fd, sizeof(struct pack_header), SEEK_SET) == -1)
+   die_errno(unable to seek in reused packfile);
 
if (reuse_packfile_offset  0)
reuse_packfile_offset = reuse_packfile-pack_size - 20;
@@ -736,10 +730,8 @@ static off_t write_reused_pack(struct sha1file *f)
while (to_write) {
int read_pack = xread(fd, buffer, sizeof(buffer));
 
-   if (read_pack = 0) {
-   close(fd);
-   return 0;
-   }
+   if (read_pack = 0)
+   die_errno(unable to read from reused packfile);
 
if (read_pack  to_write)
read_pack = to_write;
@@ -785,9 +777,6 @@ static void write_pack_file(void)
assert(pack_to_stdout);
 
packfile_size = write_reused_pack(f);
-   if (!packfile_size)
-   die_errno(failed to re-use existing pack);
-
offset += packfile_size;
}
 
@@ -909,86 +898,143 @@ static int no_try_delta(const char *path)
return 0;
 }
 
-static int add_object_entry_1(const unsigned char *sha1, enum object_type type,
- int flags, uint32_t name_hash,
- struct packed_git *found_pack, off_t found_offset)
+/*
+ * When adding an object, check whether we have already added it
+ * to our packing list. If so, we can skip. However, if we are
+ * being 

Re: [PATCH v4 0/22] pack bitmaps

2013-12-21 Thread Jeff King
On Sat, Dec 21, 2013 at 08:56:51AM -0500, Jeff King wrote:

 Interdiff is below.
 
   [01/23]: sha1write: make buffer const-correct
   [02/23]: revindex: Export new APIs
   [03/23]: pack-objects: Refactor the packing list
   [04/23]: pack-objects: factor out name_hash
   [05/23]: revision: allow setting custom limiter function
   [06/23]: sha1_file: export `git_open_noatime`
   [07/23]: compat: add endianness helpers
   [08/23]: ewah: compressed bitmap implementation
   [09/23]: documentation: add documentation for the bitmap format

By the way, the patches are identical up through 09/23. I think the
first one is already merged into another topic, too, so it may be worth
building on that instead of re-applying.

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


Re: [PATCH v4 0/22] pack bitmaps

2013-12-21 Thread Jeff King
On Sat, Dec 21, 2013 at 08:56:51AM -0500, Jeff King wrote:

 The changes from v3 are:
 
  - reworked add_object_entry refactoring (see patch 11, which is new,
and patch 12 which builds on it in a more natural way)
 
  - better error/die reporting from write_reused_pack
 
  - added Ramsay's PRIx64 compat fix
 
  - fixed a user-after-free in the warning message of open_pack_bitmap_1
 
  - minor typo/thinko fixes from Thomas in docs and tests

One thing explicitly _not_ here is ripping out khash in favor of
Karsten's hash system. That is still on the table, but I'd much rather
do it on top if we are going to.

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