Skip to content

Commit 176a65e

Browse files
pks-tgitster
authored andcommitted
object-store: remove global array of cached objects
Cached objects are virtual objects that can be set up without writing anything into the object store directly, which is used by git-blame(1) to create fake commits for the working tree. These cached objects are stored in a global variable, which is another roadblock for libification of the object subsystem. Refactor the code so that we instead store the array as part of the raw object store. This refactoring raises the question whether virtual objects should really be specific to a single repository (or rather a single object store). Hypothetical usecases might for example span across submodules, and here it may or may not be the right thing to provide virtual objects across submodule boundaries. The only existing usecase is git-blame(1) though, which does not know to blame across submodule boundaries in the first place. As such, storing these objects both globally and per-repository would achieve the same result right now. But arguably, if we learned to blame across submodule boundaries, we would likely want to create separate fare working tree commits for each of the submodules so that the user can learn which worktree a specific uncommitted change belongs to. And even if we would want to create the same fake commit for each of the submodules we could do that when storing separate virtual objects per object store. While this is all rather hypothetical, the takeaway is that handling virtual objects per-object store gives us more flexibility compared to storing them globally. In a hypothetical future where we have achieved full libification one might be able to handle unrelated repositories in a single process, where the state of one repository should not have an impact on the state of another repository. As such, storing these cached objects per object store will enable more usecases and should lead to less surprising outcomes overall. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a36d513 commit 176a65e

File tree

3 files changed

+37
-18
lines changed

3 files changed

+37
-18
lines changed

blame.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ static struct commit *fake_working_tree_commit(struct repository *r,
277277
convert_to_git(r->index, path, buf.buf, buf.len, &buf, 0);
278278
origin->file.ptr = buf.buf;
279279
origin->file.size = buf.len;
280-
pretend_object_file(buf.buf, buf.len, OBJ_BLOB, &origin->blob_oid);
280+
pretend_object_file(the_repository, buf.buf, buf.len, OBJ_BLOB, &origin->blob_oid);
281281

282282
/*
283283
* Read the current index, replace the path entry with

object-store-ll.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ static inline int pack_map_entry_cmp(const void *cmp_data UNUSED,
151151
return strcmp(pg1->pack_name, key ? key : pg2->pack_name);
152152
}
153153

154+
struct cached_object_entry;
155+
154156
struct raw_object_store {
155157
/*
156158
* Set of all object directories; the main directory is first (and
@@ -203,6 +205,15 @@ struct raw_object_store {
203205
unsigned flags;
204206
} kept_pack_cache;
205207

208+
/*
209+
* This is meant to hold a *small* number of objects that you would
210+
* want repo_read_object_file() to be able to return, but yet you do not want
211+
* to write them into the object store (e.g. a browse-only
212+
* application).
213+
*/
214+
struct cached_object_entry *cached_objects;
215+
size_t cached_object_nr, cached_object_alloc;
216+
206217
/*
207218
* A map of packfiles to packed_git structs for tracking which
208219
* packs have been loaded already.
@@ -272,7 +283,8 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
272283
* object in persistent storage before writing any other new objects
273284
* that reference it.
274285
*/
275-
int pretend_object_file(void *, unsigned long, enum object_type,
286+
int pretend_object_file(struct repository *repo,
287+
void *buf, unsigned long len, enum object_type type,
276288
struct object_id *oid);
277289

278290
struct object_info {

object-store.c

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,31 +30,31 @@
3030
* to write them into the object store (e.g. a browse-only
3131
* application).
3232
*/
33-
static struct cached_object_entry {
33+
struct cached_object_entry {
3434
struct object_id oid;
3535
struct cached_object {
3636
enum object_type type;
3737
const void *buf;
3838
unsigned long size;
3939
} value;
40-
} *cached_objects;
41-
static int cached_object_nr, cached_object_alloc;
40+
};
4241

43-
static const struct cached_object *find_cached_object(const struct object_id *oid)
42+
static const struct cached_object *find_cached_object(struct raw_object_store *object_store,
43+
const struct object_id *oid)
4444
{
4545
static const struct cached_object empty_tree = {
4646
.type = OBJ_TREE,
4747
.buf = "",
4848
};
49-
int i;
50-
const struct cached_object_entry *co = cached_objects;
49+
const struct cached_object_entry *co = object_store->cached_objects;
5150

52-
for (i = 0; i < cached_object_nr; i++, co++) {
51+
for (size_t i = 0; i < object_store->cached_object_nr; i++, co++)
5352
if (oideq(&co->oid, oid))
5453
return &co->value;
55-
}
56-
if (oideq(oid, the_hash_algo->empty_tree))
54+
55+
if (oid->algo && oideq(oid, hash_algos[oid->algo].empty_tree))
5756
return &empty_tree;
57+
5858
return NULL;
5959
}
6060

@@ -650,7 +650,7 @@ static int do_oid_object_info_extended(struct repository *r,
650650
if (!oi)
651651
oi = &blank_oi;
652652

653-
co = find_cached_object(real);
653+
co = find_cached_object(r->objects, real);
654654
if (co) {
655655
if (oi->typep)
656656
*(oi->typep) = co->type;
@@ -853,18 +853,21 @@ int oid_object_info(struct repository *r,
853853
return type;
854854
}
855855

856-
int pretend_object_file(void *buf, unsigned long len, enum object_type type,
856+
int pretend_object_file(struct repository *repo,
857+
void *buf, unsigned long len, enum object_type type,
857858
struct object_id *oid)
858859
{
859860
struct cached_object_entry *co;
860861
char *co_buf;
861862

862-
hash_object_file(the_hash_algo, buf, len, type, oid);
863-
if (repo_has_object_file_with_flags(the_repository, oid, OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT) ||
864-
find_cached_object(oid))
863+
hash_object_file(repo->hash_algo, buf, len, type, oid);
864+
if (repo_has_object_file_with_flags(repo, oid, OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT) ||
865+
find_cached_object(repo->objects, oid))
865866
return 0;
866-
ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
867-
co = &cached_objects[cached_object_nr++];
867+
868+
ALLOC_GROW(repo->objects->cached_objects,
869+
repo->objects->cached_object_nr + 1, repo->objects->cached_object_alloc);
870+
co = &repo->objects->cached_objects[repo->objects->cached_object_nr++];
868871
co->value.size = len;
869872
co->value.type = type;
870873
co_buf = xmalloc(len);
@@ -1021,6 +1024,10 @@ void raw_object_store_clear(struct raw_object_store *o)
10211024
o->odb_tail = NULL;
10221025
o->loaded_alternates = 0;
10231026

1027+
for (size_t i = 0; i < o->cached_object_nr; i++)
1028+
free((char *) o->cached_objects[i].value.buf);
1029+
FREE_AND_NULL(o->cached_objects);
1030+
10241031
INIT_LIST_HEAD(&o->packed_git_mru);
10251032
close_object_store(o);
10261033

0 commit comments

Comments
 (0)