Skip to content

Commit

Permalink
Merge gvfs-block-alloc
Browse files Browse the repository at this point in the history
Teach git the ability to allocate a block of memory upfront and manually
manage it

Git often needs to manage the memory for many instances of small
structs, for example cache_entrys. It allocates memory for each instance
of a cache_entry struct individually. While this makes it easier to
manage, repeatedly allocating small amounts of memory performance
implications. It might be more efficient to allocate a large chunk of
memory and manage instances from this block. This improved performance
can come with restrictions in how the memory is managed.

Overview

This change has several parts:

-   Move the existing memory pool type into a more generalized and
    accessible structure

-   Add an API to allocate / free cache_entry instances and have all
    cache_entry allocations go through this API.

-   Have the cache_entry allocation API allocate memory from a mem_pool
    as appropriate.

Split Index Overview

One challenge is handling memory management, especially in the
presence of split index mode. In split index mode, the majority of the
cache entries are stored in a separate file on disk (referred to as
the split index file) while the $GIT_DIR/index file stores
modifications to apply.

In Split index mode, there are 2 related constructs:

-   The on disk storage of the index:

-   $GIT_DIR/index: stores modifications to apply to the entries in
    the split index file

-   Split index file: stores the majority of the cache entries.

-   The in memory representation of the index:

-   the_index: Represents the logical view of the index.

-   split_index: Represents the view of the "base" cache entries that
    will be written to the split index file.

Cache Entry Lifetime

There was not a strong notion about who owns the lifetimes of these
objects. These objects are not ref counted and can be referenced by
both the_index and the split_index. The lifetime of these objects
was managed by following rules:

-   the_index can reference a single split_index.

-   the_index can reference cache_entrys linked to the split_index

-   the split_index can be referenced by multiple other indexs, and is
    ref counted.

-   the_index is discarded before the corresponding split_index

-   When discarding the_index, it will free cache_entries that it
    refers to as long as the cache_entry is not linked with the split
    index

Memory Pool Design

We introduce the concept of a memory pool from which cache entries
will be allocated. Every instance of an index will have an associated
memory pool. A memory pool is associated with a single index. When an
index is discarded, the associated memory pool and its allocated
cache_entrys are freed. The lifetime of a memory pool and
the associated cache_entries are tied to an index. (Note, in some
special cases, we can move the memory pool to another index).

An index can reference cache_entries allocated by its memory pool, or
by the memory pool of the associated split index. The split index has
a longer lifetime than the_index. The split_index can be shared with
multiple "parent" indexes This layering, where cache_entries can be
shared by a split_index to a "parent" index, but not the other way
around) means we can free a memory pool and the cache entries
allocated from it when we discard the associated index.

When allocating cache_entrys in split index mode, they need to be
allocated from the memory pool associated with the split index, as
these cache entries can be applied to both the the_index and the
split_index.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
  • Loading branch information
dscho committed Mar 29, 2018
2 parents 971be05 + b662e0b commit 1b9d848
Show file tree
Hide file tree
Showing 18 changed files with 474 additions and 153 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ LIB_OBJS += lockfile.o
LIB_OBJS += log-tree.o
LIB_OBJS += mailinfo.o
LIB_OBJS += mailmap.o
LIB_OBJS += mem-pool.o
LIB_OBJS += match-trees.o
LIB_OBJS += merge.o
LIB_OBJS += merge-blobs.o
Expand Down
26 changes: 12 additions & 14 deletions apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -4108,12 +4108,12 @@ static int build_fake_ancestor(struct apply_state *state, struct patch *list)
return error(_("sha1 information is lacking or useless "
"(%s)."), name);

ce = make_cache_entry(patch->old_mode, oid.hash, name, 0, 0);
ce = make_cache_entry_from_index(&result, patch->old_mode, oid.hash, name, 0, 0);
if (!ce)
return error(_("make_cache_entry failed for path '%s'"),
return error(_("make_cache_entry_from_index failed for path '%s'"),
name);
if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD)) {
free(ce);
cache_entry_free(ce);
return error(_("could not add %s to temporary index"),
name);
}
Expand Down Expand Up @@ -4281,12 +4281,11 @@ static int add_index_file(struct apply_state *state,
struct stat st;
struct cache_entry *ce;
int namelen = strlen(path);
unsigned ce_size = cache_entry_size(namelen);

if (!state->update_index)
return 0;

ce = xcalloc(1, ce_size);
ce = make_empty_cache_entry(namelen);
memcpy(ce->name, path, namelen);
ce->ce_mode = create_ce_mode(mode);
ce->ce_flags = create_ce_flags(0);
Expand All @@ -4296,27 +4295,27 @@ static int add_index_file(struct apply_state *state,

if (!skip_prefix(buf, "Subproject commit ", &s) ||
get_oid_hex(s, &ce->oid)) {
free(ce);
return error(_("corrupt patch for submodule %s"), path);
cache_entry_free(ce);
return error(_("corrupt patch for submodule %s"), path);
}
} else {
if (!state->cached) {
if (lstat(path, &st) < 0) {
free(ce);
cache_entry_free(ce);
return error_errno(_("unable to stat newly "
"created file '%s'"),
path);
}
fill_stat_cache_info(ce, &st);
}
if (write_sha1_file(buf, size, blob_type, ce->oid.hash) < 0) {
free(ce);
cache_entry_free(ce);
return error(_("unable to create backing store "
"for newly created file %s"), path);
}
}
if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
free(ce);
cache_entry_free(ce);
return error(_("unable to add cache entry for %s"), path);
}

Expand Down Expand Up @@ -4440,27 +4439,26 @@ static int add_conflicted_stages_file(struct apply_state *state,
struct patch *patch)
{
int stage, namelen;
unsigned ce_size, mode;
unsigned mode;
struct cache_entry *ce;

if (!state->update_index)
return 0;
namelen = strlen(patch->new_name);
ce_size = cache_entry_size(namelen);
mode = patch->new_mode ? patch->new_mode : (S_IFREG | 0644);

remove_file_from_cache(patch->new_name);
for (stage = 1; stage < 4; stage++) {
if (is_null_oid(&patch->threeway_stage[stage - 1]))
continue;
ce = xcalloc(1, ce_size);
ce = make_empty_cache_entry(namelen);
memcpy(ce->name, patch->new_name, namelen);
ce->ce_mode = create_ce_mode(mode);
ce->ce_flags = create_ce_flags(stage);
ce->ce_namelen = namelen;
oidcpy(&ce->oid, &patch->threeway_stage[stage - 1]);
if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
free(ce);
cache_entry_free(ce);
return error(_("unable to add cache entry for %s"),
patch->new_name);
}
Expand Down
5 changes: 2 additions & 3 deletions blame.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
struct strbuf buf = STRBUF_INIT;
const char *ident;
time_t now;
int size, len;
int len;
struct cache_entry *ce;
unsigned mode;
struct strbuf msg = STRBUF_INIT;
Expand Down Expand Up @@ -252,8 +252,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
/* Let's not bother reading from HEAD tree */
mode = S_IFREG | 0644;
}
size = cache_entry_size(len);
ce = xcalloc(1, size);
ce = make_empty_cache_entry(len);
oidcpy(&ce->oid, &origin->blob_oid);
memcpy(ce->name, path, len);
ce->ce_flags = create_ce_flags(0);
Expand Down
8 changes: 4 additions & 4 deletions builtin/checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ static int update_some(const unsigned char *sha1, struct strbuf *base,
return READ_TREE_RECURSIVE;

len = base->len + strlen(pathname);
ce = xcalloc(1, cache_entry_size(len));
ce = make_empty_cache_entry(len);
hashcpy(ce->oid.hash, sha1);
memcpy(ce->name, base->buf, base->len);
memcpy(ce->name + base->len, pathname, len - base->len);
Expand All @@ -101,7 +101,7 @@ static int update_some(const unsigned char *sha1, struct strbuf *base,
if (ce->ce_mode == old->ce_mode &&
!oidcmp(&ce->oid, &old->oid)) {
old->ce_flags |= CE_UPDATE;
free(ce);
cache_entry_free(ce);
return 0;
}
}
Expand Down Expand Up @@ -236,11 +236,11 @@ static int checkout_merged(int pos, const struct checkout *state)
blob_type, oid.hash))
die(_("Unable to add merge result for '%s'"), path);
free(result_buf.ptr);
ce = make_cache_entry(mode, oid.hash, path, 2, 0);
ce = make_transient_cache_entry(mode, oid.hash, path, 2);
if (!ce)
die(_("make_cache_entry failed for path '%s'"), path);
status = checkout_entry(ce, state, NULL);
free(ce);
transient_cache_entry_free(ce);
return status;
}

Expand Down
6 changes: 3 additions & 3 deletions builtin/difftool.c
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,10 @@ static int checkout_path(unsigned mode, struct object_id *oid,
struct cache_entry *ce;
int ret;

ce = make_cache_entry(mode, oid->hash, path, 0, 0);
ce = make_transient_cache_entry(mode, oid->hash, path, 0);
ret = checkout_entry(ce, state, NULL);

free(ce);
transient_cache_entry_free(ce);
return ret;
}

Expand Down Expand Up @@ -488,7 +488,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
* index.
*/
struct cache_entry *ce2 =
make_cache_entry(rmode, roid.hash,
make_cache_entry_from_index(&wtindex, rmode, roid.hash,
dst_path, 0, 0);

add_index_entry(&wtindex, ce2,
Expand Down
8 changes: 4 additions & 4 deletions builtin/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ static void update_index_from_diff(struct diff_queue_struct *q,
state.force = 1;
state.refresh_cache = 1;
state.istate = &the_index;
ceBefore = make_cache_entry(two->mode, two->oid.hash, two->path,
ceBefore = make_cache_entry_from_index(&the_index, two->mode, two->oid.hash, two->path,
0, 0);
if (!ceBefore)
die(_("make_cache_entry failed for path '%s'"),
die(_("make_cache_entry_from_index failed for path '%s'"),
two->path);

checkout_entry(ceBefore, &state, NULL);
Expand All @@ -179,10 +179,10 @@ static void update_index_from_diff(struct diff_queue_struct *q,
continue;
}

ce = make_cache_entry(one->mode, one->oid.hash, one->path,
ce = make_cache_entry_from_index(&the_index, one->mode, one->oid.hash, one->path,
0, 0);
if (!ce)
die(_("make_cache_entry failed for path '%s'"),
die(_("make_cache_entry_from_index failed for path '%s'"),
one->path);
if (is_missing) {
ce->ce_flags |= CE_INTENT_TO_ADD;
Expand Down
26 changes: 11 additions & 15 deletions builtin/update-index.c
Original file line number Diff line number Diff line change
Expand Up @@ -268,15 +268,14 @@ static int process_lstat_error(const char *path, int err)

static int add_one_path(const struct cache_entry *old, const char *path, int len, struct stat *st)
{
int option, size;
int option;
struct cache_entry *ce;

/* Was the old index entry already up-to-date? */
if (old && !ce_stage(old) && !ce_match_stat(old, st, 0))
return 0;

size = cache_entry_size(len);
ce = xcalloc(1, size);
ce = make_empty_cache_entry(len);
memcpy(ce->name, path, len);
ce->ce_flags = create_ce_flags(0);
ce->ce_namelen = len;
Expand All @@ -285,13 +284,13 @@ static int add_one_path(const struct cache_entry *old, const char *path, int len

if (index_path(&ce->oid, path, st,
info_only ? 0 : HASH_WRITE_OBJECT)) {
free(ce);
cache_entry_free(ce);
return -1;
}
option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
if (add_cache_entry(ce, option)) {
free(ce);
cache_entry_free(ce);
return error("%s: cannot add to the index - missing --add option?", path);
}
return 0;
Expand Down Expand Up @@ -403,15 +402,14 @@ static int process_path(const char *path)
static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
const char *path, int stage)
{
int size, len, option;
int len, option;
struct cache_entry *ce;

if (!verify_path(path))
return error("Invalid path '%s'", path);

len = strlen(path);
size = cache_entry_size(len);
ce = xcalloc(1, size);
ce = make_empty_cache_entry(len);

oidcpy(&ce->oid, oid);
memcpy(ce->name, path, len);
Expand Down Expand Up @@ -589,7 +587,6 @@ static struct cache_entry *read_one_ent(const char *which,
{
unsigned mode;
struct object_id oid;
int size;
struct cache_entry *ce;

if (get_tree_entry(ent->hash, path, oid.hash, &mode)) {
Expand All @@ -602,8 +599,7 @@ static struct cache_entry *read_one_ent(const char *which,
error("%s: not a blob in %s branch.", path, which);
return NULL;
}
size = cache_entry_size(namelen);
ce = xcalloc(1, size);
ce = make_empty_cache_entry(namelen);

oidcpy(&ce->oid, &oid);
memcpy(ce->name, path, namelen);
Expand Down Expand Up @@ -680,8 +676,8 @@ static int unresolve_one(const char *path)
error("%s: cannot add their version to the index.", path);
ret = -1;
free_return:
free(ce_2);
free(ce_3);
cache_entry_free(ce_2);
cache_entry_free(ce_3);
return ret;
}

Expand Down Expand Up @@ -748,7 +744,7 @@ static int do_reupdate(int ac, const char **av,
ce->name, ce_namelen(ce), 0);
if (old && ce->ce_mode == old->ce_mode &&
!oidcmp(&ce->oid, &old->oid)) {
free(old);
cache_entry_free(old);
continue; /* unchanged */
}
/* Be careful. The working tree may not have the
Expand All @@ -759,7 +755,7 @@ static int do_reupdate(int ac, const char **av,
path = xstrdup(ce->name);
update_one(path);
free(path);
free(old);
cache_entry_free(old);
if (save_nr != active_nr)
goto redo;
}
Expand Down
30 changes: 28 additions & 2 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "convert.h"
#include "trace.h"
#include "string-list.h"
#include "mem-pool.h"
#include "pack-revindex.h"
#include "hash.h"
#include "path.h"
Expand Down Expand Up @@ -353,6 +354,7 @@ struct index_state {
struct untracked_cache *untracked;
uint64_t fsmonitor_last_update;
struct ewah_bitmap *fsmonitor_dirty;
struct mem_pool *ce_mem_pool;
};

extern struct index_state the_index;
Expand Down Expand Up @@ -396,6 +398,7 @@ extern void free_name_hash(struct index_state *istate);
#define unmerge_cache_entry_at(at) unmerge_index_entry_at(&the_index, at)
#define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
#define read_blob_data_from_cache(path, sz) read_blob_data_from_index(&the_index, (path), (sz))
#define make_empty_cache_entry(len) make_empty_cache_entry_from_index(&the_index, len)
#endif

enum object_type {
Expand Down Expand Up @@ -712,7 +715,25 @@ extern int remove_file_from_index(struct index_state *, const char *path);
extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
extern int add_file_to_index(struct index_state *, const char *path, int flags);

extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
extern struct cache_entry *make_cache_entry_from_index(struct index_state *, unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
extern struct cache_entry *make_transient_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage);

/*
* Create an empty cache entry struct. This is intended for use with
* cache_entries that could be added to the associated index.
*
* istate: The index whose memory pool this cache entry should be allocated from.
* len: The length to reserve for the path field of the cache entry.
*/
extern struct cache_entry *make_empty_cache_entry_from_index(struct index_state *istate, size_t len);

/*
* Create an empty cache entry struct. This is intended for use with
* cache_entries that will not be added to an index.
*
* len: The length to reserve for the path field of the cache entry.
*/
extern struct cache_entry *make_empty_transient_cache_entry(size_t len);
extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
Expand Down Expand Up @@ -765,7 +786,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
#define REFRESH_IGNORE_SUBMODULES 0x0010 /* ignore submodules */
#define REFRESH_IN_PORCELAIN 0x0020 /* user friendly output, not "needs update" */
extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);
extern struct cache_entry *refresh_cache_entry(struct cache_entry *, unsigned int);
extern struct cache_entry *refresh_cache_entry(struct index_state *, struct cache_entry *, unsigned int);

/*
* Opportunistically update the index but do not complain if we can't.
Expand Down Expand Up @@ -1988,4 +2009,9 @@ void safe_create_dir(const char *dir, int share);
*/
extern int print_sha1_ellipsis(void);

void cache_entry_free(struct cache_entry *ce);
void transient_cache_entry_free(struct cache_entry *ce);

void validate_cache_entries(const struct index_state *istate);

#endif /* CACHE_H */
Loading

0 comments on commit 1b9d848

Please sign in to comment.