"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: tog is sluggish with FreeBSD src.git
To:
Christian Weisgerber <naddy@mips.inka.de>, gameoftrees@openbsd.org
Date:
Fri, 25 Dec 2020 18:39:02 +0100

Download raw body.

Thread
On Fri, Dec 25, 2020 at 12:38:23AM +0100, Stefan Sperling wrote:
> On Thu, Dec 24, 2020 at 04:39:29PM +0100, Christian Weisgerber wrote:
> > Stefan Sperling:
> > > It looks like we'll need to populate a cache of reference names which
> > > is indexed by commit ID, ideally while reading references from the repo?
> > 
> > got_ref_tree()?  Something along those lines, I guess.
> 
> Can you try this? Merry christmas <3

Here is another patch (which applies on top of the previous patch) that
shows how we can avoid the delay caused by reading references from disk
when opening a diff view from the log view.

The same idea could be applied to other views, e.g. the blame view
could also pass a list of references to the diff view.

Once this has spread to all tog views, reading the reference list of the
FreeBSD repo becomes a 1x start-up cost for tog. We could even optimize
got_ref_list() itself if this start-up overhead is not acceptable.
The got_ref_list() function currently traverses the entire list every
time a new ref is added, which is obviously suboptimal.

diff b1700e6f9a5e50cb46002b7775c0a5ac9059b6fc refs/heads/object_id_map
blob - 8a7ddd1fed280c5ace7be5e759d1b7ef2c6cc784
blob + 86bb696e9930246f8970ea7067581e1b5751136e
--- include/got_reference.h
+++ include/got_reference.h
@@ -110,6 +110,13 @@ const struct got_error *got_ref_list(struct got_reflis
 /* Free all references on a ref list. */
 void got_ref_list_free(struct got_reflist_head *);
 
+/*
+ * Deep-copy all elements of a reference list to another reference list.
+ * The destination list must already have been initialized with SIMPLEQ_INIT.
+ */
+const struct got_error *got_ref_list_copy(struct got_reflist_head *,
+    struct got_reflist_head *);
+
 /* Indicate whether the provided reference is symbolic (points at another
  * refernce) or not (points at an object ID). */
 int got_ref_is_symbolic(struct got_reference *);
blob - d9f1596591be3332b0a51e1ec4e6c77a90720fae
blob + b84f193dae6dedc72bf14c332337b51f036cbfb6
--- lib/reference.c
+++ lib/reference.c
@@ -1021,6 +1021,24 @@ got_ref_list_free(struct got_reflist_head *refs)
 
 }
 
+const struct got_error *
+got_ref_list_copy(struct got_reflist_head *src, struct got_reflist_head *dst)
+{
+	const struct got_error *err = NULL;
+	struct got_reflist_entry *re, *new;
+
+	SIMPLEQ_FOREACH(re, src, entry) {
+		err = got_reflist_entry_dup(&new, re);
+		if (err) {
+			got_ref_list_free(dst);
+			return err;
+		}
+		SIMPLEQ_INSERT_TAIL(dst, new, entry);
+	}
+
+	return NULL;
+}
+
 int
 got_ref_is_symbolic(struct got_reference *ref)
 {
blob - a28a00571d687d94f4f9215df59a0fe82423e7c9
blob + 68bd070d1013a0af0dd554d233924ced3235b42e
--- tog/tog.c
+++ tog/tog.c
@@ -495,7 +495,7 @@ struct tog_view {
 static const struct got_error *open_diff_view(struct tog_view *,
     struct got_object_id *, struct got_object_id *,
     const char *, const char *, int, int, int, struct tog_view *,
-    struct got_repository *);
+    struct got_reflist_head *, struct got_repository *);
 static const struct got_error *show_diff_view(struct tog_view *);
 static const struct got_error *input_diff_view(struct tog_view **,
     struct tog_view *, int);
@@ -1820,7 +1820,8 @@ log_scroll_down(struct tog_view *view, int maxscroll)
 static const struct got_error *
 open_diff_view_for_commit(struct tog_view **new_view, int begin_x,
     struct got_commit_object *commit, struct got_object_id *commit_id,
-    struct tog_view *log_view, struct got_repository *repo)
+    struct tog_view *log_view, struct got_reflist_head *refs,
+    struct got_repository *repo)
 {
 	const struct got_error *err;
 	struct got_object_qid *parent_id;
@@ -1832,7 +1833,7 @@ open_diff_view_for_commit(struct tog_view **new_view, 
 
 	parent_id = SIMPLEQ_FIRST(got_object_commit_get_parent_ids(commit));
 	err = open_diff_view(diff_view, parent_id ? parent_id->id : NULL,
-	    commit_id, NULL, NULL, 3, 0, 0, log_view, repo);
+	    commit_id, NULL, NULL, 3, 0, 0, log_view, refs, repo);
 	if (err == NULL)
 		*new_view = diff_view;
 	return err;
@@ -2474,7 +2475,7 @@ input_log_view(struct tog_view **new_view, struct tog_
 			begin_x = view_split_begin_x(view->begin_x);
 		err = open_diff_view_for_commit(&diff_view, begin_x,
 		    s->selected_entry->commit, s->selected_entry->id,
-		    view, s->repo);
+		    view, &s->refs, s->repo);
 		if (err)
 			break;
 		view->focussed = 0;
@@ -3409,7 +3410,8 @@ static const struct got_error *
 open_diff_view(struct tog_view *view, struct got_object_id *id1,
     struct got_object_id *id2, const char *label1, const char *label2,
     int diff_context, int ignore_whitespace, int force_text_diff,
-    struct tog_view *log_view, struct got_repository *repo)
+    struct tog_view *log_view, struct got_reflist_head *refs,
+    struct got_repository *repo)
 {
 	const struct got_error *err;
 	struct tog_diff_view_state *s = &view->state.diff;
@@ -3507,7 +3509,11 @@ open_diff_view(struct tog_view *view, struct got_objec
 		}
 	}
 
-	err = got_ref_list(&s->refs, repo, NULL, got_ref_cmp_by_name, NULL);
+	if (refs)
+		err = got_ref_list_copy(refs, &s->refs);
+	else
+		err = got_ref_list(&s->refs, repo, NULL, got_ref_cmp_by_name,
+		    NULL);
 	if (err) {
 		free(s->id1);
 		s->id1 = NULL;
@@ -3871,7 +3877,7 @@ cmd_diff(int argc, char *argv[])
 		goto done;
 	}
 	error = open_diff_view(view, id1, id2, label1, label2, diff_context,
-	    ignore_whitespace, force_text_diff, NULL,  repo);
+	    ignore_whitespace, force_text_diff, NULL, NULL, repo);
 	if (error)
 		goto done;
 	error = view_loop(view);
@@ -4624,7 +4630,7 @@ input_blame_view(struct tog_view **new_view, struct to
 			break;
 		}
 		err = open_diff_view(diff_view, pid ? pid->id : NULL,
-		    id, NULL, NULL, 3, 0, 0, NULL, s->repo);
+		    id, NULL, NULL, 3, 0, 0, NULL, NULL, s->repo);
 		got_object_commit_close(commit);
 		if (err) {
 			view_close(diff_view);