Merge branch 'kw/write-index-reduce-alloc'
authorJunio C Hamano <gitster@pobox.com>
Sun, 27 Aug 2017 05:55:08 +0000 (22:55 -0700)
committerJunio C Hamano <gitster@pobox.com>
Sun, 27 Aug 2017 05:55:08 +0000 (22:55 -0700)
We used to spend more than necessary cycles allocating and freeing
piece of memory while writing each index entry out. This has been
optimized.

* kw/write-index-reduce-alloc:
read-cache: avoid allocating every ondisk entry when writing
read-cache: fix memory leak in do_write_index
perf: add test for writing the index

1  2 
Makefile
read-cache.c
diff --combined Makefile
index 493b8f5d2da278b01a5e051b68d621070737072c,6e9b80751e274ba622aad269ecd987e32c3a9bad..ffab6f456842a911f86b93a72e0cc342341b9c3c
+++ b/Makefile
@@@ -655,6 -655,7 +655,7 @@@ TEST_PROGRAMS_NEED_X += test-parse-opti
  TEST_PROGRAMS_NEED_X += test-path-utils
  TEST_PROGRAMS_NEED_X += test-prio-queue
  TEST_PROGRAMS_NEED_X += test-read-cache
+ TEST_PROGRAMS_NEED_X += test-write-cache
  TEST_PROGRAMS_NEED_X += test-ref-store
  TEST_PROGRAMS_NEED_X += test-regex
  TEST_PROGRAMS_NEED_X += test-revision-walking
@@@ -842,7 -843,6 +843,7 @@@ LIB_OBJS += reflog-walk.
  LIB_OBJS += refs.o
  LIB_OBJS += refs/files-backend.o
  LIB_OBJS += refs/iterator.o
 +LIB_OBJS += refs/packed-backend.o
  LIB_OBJS += refs/ref-cache.o
  LIB_OBJS += ref-filter.o
  LIB_OBJS += remote.o
@@@ -2038,6 -2038,7 +2039,6 @@@ XDIFF_OBJS += xdiff/xhistogram.
  
  VCSSVN_OBJS += vcs-svn/line_buffer.o
  VCSSVN_OBJS += vcs-svn/sliding_window.o
 -VCSSVN_OBJS += vcs-svn/repo_tree.o
  VCSSVN_OBJS += vcs-svn/fast_export.o
  VCSSVN_OBJS += vcs-svn/svndiff.o
  VCSSVN_OBJS += vcs-svn/svndump.o
diff --combined read-cache.c
index 9b4105856992859f3a1d7f4462843196aacdc484,694bed8d82807fccc235cecf5e8d711a1e3d83b1..40da87ea71f088114c5a40893850cc88efee6953
@@@ -160,9 -160,9 +160,9 @@@ static int ce_compare_data(const struc
        int fd = git_open_cloexec(ce->name, O_RDONLY);
  
        if (fd >= 0) {
 -              unsigned char sha1[20];
 -              if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
 -                      match = hashcmp(sha1, ce->oid.hash);
 +              struct object_id oid;
 +              if (!index_fd(&oid, fd, st, OBJ_BLOB, ce->name, 0))
 +                      match = oidcmp(&oid, &ce->oid);
                /* index_fd() closed the file descriptor already */
        }
        return match;
@@@ -689,7 -689,7 +689,7 @@@ int add_to_index(struct index_state *is
                return 0;
        }
        if (!intent_only) {
 -              if (index_path(ce->oid.hash, path, st, HASH_WRITE_OBJECT)) {
 +              if (index_path(&ce->oid, path, st, HASH_WRITE_OBJECT)) {
                        free(ce);
                        return error("unable to index file %s", path);
                }
@@@ -1499,6 -1499,7 +1499,7 @@@ struct ondisk_cache_entry_extended 
  };
  
  /* These are only used for v3 or lower */
+ #define align_padding_size(size, len) ((size + (len) + 8) & ~7) - (size + len)
  #define align_flex_name(STRUCT,len) ((offsetof(struct STRUCT,name) + (len) + 8) & ~7)
  #define ondisk_cache_entry_size(len) align_flex_name(ondisk_cache_entry,len)
  #define ondisk_cache_entry_extended_size(len) align_flex_name(ondisk_cache_entry_extended,len)
@@@ -2032,7 -2033,7 +2033,7 @@@ static void ce_smudge_racily_clean_entr
  }
  
  /* Copy miscellaneous fields but not the name */
- static char *copy_cache_entry_to_ondisk(struct ondisk_cache_entry *ondisk,
+ static void copy_cache_entry_to_ondisk(struct ondisk_cache_entry *ondisk,
                                       struct cache_entry *ce)
  {
        short flags;
                struct ondisk_cache_entry_extended *ondisk2;
                ondisk2 = (struct ondisk_cache_entry_extended *)ondisk;
                ondisk2->flags2 = htons((ce->ce_flags & CE_EXTENDED_FLAGS) >> 16);
-               return ondisk2->name;
-       }
-       else {
-               return ondisk->name;
        }
  }
  
  static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,
-                         struct strbuf *previous_name)
+                         struct strbuf *previous_name, struct ondisk_cache_entry *ondisk)
  {
        int size;
-       struct ondisk_cache_entry *ondisk;
        int saved_namelen = saved_namelen; /* compiler workaround */
-       char *name;
        int result;
+       static unsigned char padding[8] = { 0x00 };
  
        if (ce->ce_flags & CE_STRIP_NAME) {
                saved_namelen = ce_namelen(ce);
                ce->ce_namelen = 0;
        }
  
+       if (ce->ce_flags & CE_EXTENDED)
+               size = offsetof(struct ondisk_cache_entry_extended, name);
+       else
+               size = offsetof(struct ondisk_cache_entry, name);
        if (!previous_name) {
-               size = ondisk_ce_size(ce);
-               ondisk = xcalloc(1, size);
-               name = copy_cache_entry_to_ondisk(ondisk, ce);
-               memcpy(name, ce->name, ce_namelen(ce));
+               int len = ce_namelen(ce);
+               copy_cache_entry_to_ondisk(ondisk, ce);
+               result = ce_write(c, fd, ondisk, size);
+               if (!result)
+                       result = ce_write(c, fd, ce->name, len);
+               if (!result)
+                       result = ce_write(c, fd, padding, align_padding_size(size, len));
        } else {
                int common, to_remove, prefix_size;
                unsigned char to_remove_vi[16];
                to_remove = previous_name->len - common;
                prefix_size = encode_varint(to_remove, to_remove_vi);
  
-               if (ce->ce_flags & CE_EXTENDED)
-                       size = offsetof(struct ondisk_cache_entry_extended, name);
-               else
-                       size = offsetof(struct ondisk_cache_entry, name);
-               size += prefix_size + (ce_namelen(ce) - common + 1);
-               ondisk = xcalloc(1, size);
-               name = copy_cache_entry_to_ondisk(ondisk, ce);
-               memcpy(name, to_remove_vi, prefix_size);
-               memcpy(name + prefix_size, ce->name + common, ce_namelen(ce) - common);
+               copy_cache_entry_to_ondisk(ondisk, ce);
+               result = ce_write(c, fd, ondisk, size);
+               if (!result)
+                       result = ce_write(c, fd, to_remove_vi, prefix_size);
+               if (!result)
+                       result = ce_write(c, fd, ce->name + common, ce_namelen(ce) - common + 1);
  
                strbuf_splice(previous_name, common, to_remove,
                              ce->name + common, ce_namelen(ce) - common);
                ce->ce_flags &= ~CE_STRIP_NAME;
        }
  
-       result = ce_write(c, fd, ondisk, size);
-       free(ondisk);
        return result;
  }
  
@@@ -2192,10 -2190,11 +2190,11 @@@ static int do_write_index(struct index_
        int newfd = tempfile->fd;
        git_SHA_CTX c;
        struct cache_header hdr;
-       int i, err, removed, extended, hdr_version;
+       int i, err = 0, removed, extended, hdr_version;
        struct cache_entry **cache = istate->cache;
        int entries = istate->cache_nr;
        struct stat st;
+       struct ondisk_cache_entry_extended ondisk;
        struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
        int drop_cache_tree = 0;
  
                return -1;
  
        previous_name = (hdr_version == 4) ? &previous_name_buf : NULL;
        for (i = 0; i < entries; i++) {
                struct cache_entry *ce = cache[i];
                if (ce->ce_flags & CE_REMOVE)
                        if (allow)
                                warning(msg, ce->name);
                        else
-                               return error(msg, ce->name);
+                               err = error(msg, ce->name);
  
                        drop_cache_tree = 1;
                }
-               if (ce_write_entry(&c, newfd, ce, previous_name) < 0)
-                       return -1;
+               if (ce_write_entry(&c, newfd, ce, previous_name, (struct ondisk_cache_entry *)&ondisk) < 0)
+                       err = -1;
+               if (err)
+                       break;
        }
        strbuf_release(&previous_name_buf);
  
+       if (err)
+               return err;
        /* Write extension data here */
        if (!strip_extensions && istate->split_index) {
                struct strbuf sb = STRBUF_INIT;