"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>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 25 Dec 2020 00:38:23 +0100

Download raw body.

Thread
On Thu, Dec 24, 2020 at 04:39:29PM +0100, Christian Weisgerber wrote:
> Stefan Sperling:
> 
> > The patch below shows us where the problem is: Iterating a long SIMPLEQ
> > relifst in order to decorate commit hashes with branch names is too slow.
> 
> "got log" has the same problem, although it's less noticeable.
> got.c:print_commit()

Right. This can be fixed once we have a known-good fix for tog.

> > This patch makes commits load faster but moving the selection cursor
> > in the log view is still too slow.
> 
> It's also buggy:
> * tog log -r got.git
> * first entry is shown as "main, origin/main" in the status line
> * move down
> * move up again
> * no more "main, origin/main"

Yes, let's forget about that patch.

> > 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

diff refs/heads/main refs/heads/object_id_map
blob - 5f2594f5af806ea9b99c358f35fa4e7ce16f7efb
blob + 8a7ddd1fed280c5ace7be5e759d1b7ef2c6cc784
--- include/got_reference.h
+++ include/got_reference.h
@@ -139,3 +139,28 @@ const struct got_error *got_ref_delete(struct got_refe
 
 /* Unlock a reference which was opened in locked state. */
 const struct got_error *got_ref_unlock(struct got_reference *);
+
+/* Map object IDs to references. */
+struct got_reflist_object_id_map;
+
+/*
+ * Create and populate an object ID map for a given list of references.
+ * Map entries will contain deep-copies of elements of the reflist.
+ * The caller must dispose of the map with got_reflist_object_map_free().
+ */
+const struct got_error *got_reflist_object_id_map_create(
+    struct got_reflist_object_id_map **, struct got_reflist_head *, 
+    struct got_repository *);
+
+/*
+ * Return a list of references which correspond to a given object ID.
+ * The returned list must be considered read-only.
+ * The caller must _not_ call free(3) on the returned pointer!
+ * If no references are associated with the ID, return NULL.
+ */
+struct got_reflist_head *
+got_reflist_object_id_map_lookup(struct got_reflist_object_id_map *,
+    struct got_object_id *);
+
+/* Free the specified object ID map. */
+void got_reflist_object_map_free(struct got_reflist_object_id_map *);
blob - ef042bcbd8a8763b42a65397975a53c336f74544
blob + d9f1596591be3332b0a51e1ec4e6c77a90720fae
--- lib/reference.c
+++ lib/reference.c
@@ -43,6 +43,7 @@
 #include "got_lib_delta.h"
 #include "got_lib_inflate.h"
 #include "got_lib_object.h"
+#include "got_lib_object_idset.h"
 #include "got_lib_lockfile.h"
 
 #ifndef nitems
@@ -535,8 +536,8 @@ got_ref_dup(struct got_reference *ref)
 			return NULL;
 		}
 	} else {
-		ref->ref.ref.name = strdup(ref->ref.ref.name);
-		if (ref->ref.ref.name == NULL) {
+		ret->ref.ref.name = strdup(ref->ref.ref.name);
+		if (ret->ref.ref.name == NULL) {
 			free(ret);
 			return NULL;
 		}
@@ -1400,3 +1401,97 @@ got_ref_unlock(struct got_reference *ref)
 	ref->lf = NULL;
 	return err;
 }
+
+struct got_reflist_object_id_map {
+	struct got_object_idset *idset;
+};
+
+struct got_reflist_object_id_map_entry {
+	struct got_reflist_head refs;
+};
+
+const struct got_error *
+got_reflist_object_id_map_create(struct got_reflist_object_id_map **map,
+    struct got_reflist_head *refs, struct got_repository *repo)
+{
+	const struct got_error *err = NULL;
+	struct got_object_idset *idset;
+	struct got_object_id *id = NULL;
+	struct got_reflist_entry *re;
+
+	idset = got_object_idset_alloc();
+	if (idset == NULL)
+		return got_error_from_errno("got_object_idset_alloc");
+
+	*map = malloc(sizeof(**map));
+	if (*map == NULL) {
+		got_object_idset_free(idset);
+		return got_error_from_errno("malloc");
+	}
+	(*map)->idset = idset;
+
+	SIMPLEQ_FOREACH(re, refs, entry) {
+		struct got_reflist_entry *new;
+		struct got_reflist_object_id_map_entry *ent;
+
+		err = got_ref_resolve(&id, repo, re->ref);
+		if (err)
+			goto done;
+
+		ent = got_object_idset_get(idset, id);
+		if (ent == NULL) {
+			ent = malloc(sizeof(*ent));
+			if (ent == NULL) {
+				err = got_error_from_errno("malloc");
+				goto done;
+			}
+			SIMPLEQ_INIT(&ent->refs);
+			err = got_object_idset_add(idset, id, ent);
+			if (err)
+				goto done;
+		}
+
+		err = got_reflist_entry_dup(&new, re);
+		if (err)
+			goto done;
+		SIMPLEQ_INSERT_TAIL(&ent->refs, new, entry);
+		free(id);
+		id = NULL;
+	}
+done:
+	free(id);
+	if (err) {
+		got_reflist_object_map_free(*map);
+		*map = NULL;
+	}
+	return NULL;
+}
+
+struct got_reflist_head *
+got_reflist_object_id_map_lookup(struct got_reflist_object_id_map *map,
+    struct got_object_id *id)
+{
+	struct got_reflist_object_id_map_entry *ent;
+	ent = got_object_idset_get(map->idset, id);
+	if (ent)
+		return &ent->refs;
+	return NULL;
+}
+
+static const struct got_error *
+free_id_map_entry(struct got_object_id *id, void *data, void *arg)
+{
+	struct got_reflist_object_id_map_entry *ent = data;
+
+	got_ref_list_free(&ent->refs);
+	free(ent);
+	return NULL;
+}
+
+void
+got_reflist_object_map_free(struct got_reflist_object_id_map *map)
+{
+	got_object_idset_for_each(map->idset, free_id_map_entry, NULL);
+	got_object_idset_free(map->idset);
+	free(map);
+}
blob - d3d4a63398b4c4b05caa731b093b038b0b1d9bee
blob + a28a00571d687d94f4f9215df59a0fe82423e7c9
--- tog/tog.c
+++ tog/tog.c
@@ -263,6 +263,7 @@ struct tog_diff_view_state {
 	int force_text_diff;
 	struct got_repository *repo;
 	struct got_reflist_head refs;
+	struct got_reflist_object_id_map *idmap;
 	struct tog_colors colors;
 	size_t nlines;
 	off_t *line_offsets;
@@ -304,6 +305,7 @@ struct tog_log_view_state {
 	int log_branches;
 	struct got_repository *repo;
 	struct got_reflist_head refs;
+	struct got_reflist_object_id_map *idmap;
 	struct got_object_id *start_id;
 	sig_atomic_t quit;
 	pthread_t thread;
@@ -1572,13 +1574,18 @@ draw_commits(struct tog_view *view)
 
 	if (s->selected_entry &&
 	    !(view->searching && view->search_next_done == 0)) {
+		struct got_reflist_head *refs;
 		err = got_object_id_str(&id_str, s->selected_entry->id);
 		if (err)
 			return err;
-		err = build_refs_str(&refs_str, &s->refs,
-		    s->selected_entry->id, s->repo);
-		if (err)
-			goto done;
+		refs = got_reflist_object_id_map_lookup(s->idmap,
+		    s->selected_entry->id);
+		if (refs) {
+			err = build_refs_str(&refs_str, refs,
+			    s->selected_entry->id, s->repo);
+			if (err)
+				goto done;
+		}
 	}
 
 	if (s->thread_args.commits_needed == 0)
@@ -2118,6 +2125,10 @@ close_log_view(struct tog_view *view)
 	s->start_id = NULL;
 	free(s->head_ref_name);
 	s->head_ref_name = NULL;
+	if (s->idmap) {
+		got_reflist_object_map_free(s->idmap);
+		s->idmap = NULL;
+	}
 	got_ref_list_free(&s->refs);
 	return err;
 }
@@ -2263,6 +2274,9 @@ open_log_view(struct tog_view *view, struct got_object
 	err = got_ref_list(&s->refs, repo, NULL, got_ref_cmp_by_name, NULL);
 	if (err)
 		goto done;
+	err = got_reflist_object_id_map_create(&s->idmap, &s->refs, repo);
+	if (err)
+		goto done;
 
 	s->repo = repo;
 	if (head_ref_name) {
@@ -2529,11 +2543,19 @@ input_log_view(struct tog_view **new_view, struct tog_
 		    got_repo_get_path(s->repo), NULL);
 		if (err)
 			return err;
+		if (s->idmap) {
+			got_reflist_object_map_free(s->idmap);
+			s->idmap = NULL;
+		}
 		got_ref_list_free(&s->refs);
 		err = got_ref_list(&s->refs, s->repo, NULL,
 		    got_ref_cmp_by_name, NULL);
 		if (err)
 			return err;
+		err = got_reflist_object_id_map_create(&s->idmap, &s->refs,
+		    s->repo);
+		if (err)
+			return err;
 		err = got_commit_graph_open(&s->thread_args.graph,
 		    s->in_repo_path, !s->log_branches);
 		if (err)
@@ -3254,14 +3276,16 @@ create_diff(struct tog_diff_view_state *s)
 		const struct got_object_id_queue *parent_ids;
 		struct got_object_qid *pid;
 		struct got_commit_object *commit2;
+		struct got_reflist_head *refs;
 
 		err = got_object_open_as_commit(&commit2, s->repo, s->id2);
 		if (err)
 			goto done;
+		refs = got_reflist_object_id_map_lookup(s->idmap, s->id2);
 		/* Show commit info if we're diffing to a parent/root commit. */
 		if (s->id1 == NULL) {
 			err = write_commit_info(&s->line_offsets, &s->nlines,
-			    s->id2, &s->refs, s->repo, s->f);
+			    s->id2, refs, s->repo, s->f);
 			if (err)
 				goto done;
 		} else {
@@ -3270,7 +3294,7 @@ create_diff(struct tog_diff_view_state *s)
 				if (got_object_id_cmp(s->id1, pid->id) == 0) {
 					err = write_commit_info(
 					    &s->line_offsets, &s->nlines,
-					    s->id2, &s->refs, s->repo, s->f);
+					    s->id2, refs, s->repo, s->f);
 					if (err)
 						goto done;
 					break;
@@ -3492,6 +3516,16 @@ open_diff_view(struct tog_view *view, struct got_objec
 		free_colors(&s->colors);
 		return err;
 	}
+	err = got_reflist_object_id_map_create(&s->idmap, &s->refs, repo);
+	if (err) {
+		free(s->id1);
+		s->id1 = NULL;
+		free(s->id2);
+		s->id2 = NULL;
+		free_colors(&s->colors);
+		got_ref_list_free(&s->refs);
+		return err;
+	}
 
 	if (log_view && view_is_splitscreen(view))
 		show_log_view(log_view); /* draw vborder */
@@ -3507,6 +3541,8 @@ open_diff_view(struct tog_view *view, struct got_objec
 		s->id2 = NULL;
 		free_colors(&s->colors);
 		got_ref_list_free(&s->refs);
+		got_reflist_object_map_free(s->idmap);
+		s->idmap = NULL;
 		return err;
 	}
 
@@ -3535,6 +3571,10 @@ close_diff_view(struct tog_view *view)
 	free(s->line_offsets);
 	s->line_offsets = NULL;
 	s->nlines = 0;
+	if (s->idmap) {
+		got_reflist_object_map_free(s->idmap);
+		s->idmap = NULL;
+	}
 	got_ref_list_free(&s->refs);
 	return err;
 }