Re: [PATCH] defer expensive load_ref_decorations until needed
From: Junio C Hamano <hidden>
Date: 2017-11-22 05:03:31
Phil Hord [off-list ref] writes:
quoted hunk
With many thousands of references, a simple `git rev-parse HEAD` may take more than a second to return because it first loads all the refs into memory even though it will never use them. Defer loading any references until we actually need them. Signed-off-by: Phil Hord <redacted> --- log-tree.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)diff --git a/log-tree.c b/log-tree.c index 3b904f037..c1509f8b9 100644 --- a/log-tree.c +++ b/log-tree.c@@ -84,8 +84,10 @@ void add_name_decoration(enum decoration_type type, const char *name, struct obj res->next = add_decoration(&name_decoration, obj, res); } +static void maybe_load_ref_decorations();
I'll tweak that "()" and the other one we see below to "(void)" while queuing. I am not sure if "maybe_" is a good name here, though. If anything, you are making the semantics of "load_ref_decorations()" to "maybe" (but I do not suggest renaming that one). How about calling it to load_ref_decorations_lazily() or something? I also wonder if decoration_loaded should now become function-scope static in this new helper, but that can be left outside of the topic. Other than that, I like what this patch attempts to do. A nicely identified low-hanging fruit ;-). Thanks.
quoted hunk
const struct name_decoration *get_name_decoration(const struct object *obj) { + maybe_load_ref_decorations(); return lookup_decoration(&name_decoration, obj); }@@ -150,10 +152,13 @@ static int add_graft_decoration(const struct commit_graft *graft, void *cb_data) void load_ref_decorations(int flags) { - if (!decoration_loaded) { + decoration_flags = flags; +} +static void maybe_load_ref_decorations() +{ + if (!decoration_loaded) { decoration_loaded = 1; - decoration_flags = flags; for_each_ref(add_ref_decoration, NULL); head_ref(add_ref_decoration, NULL); for_each_commit_graft(add_graft_decoration, NULL);