Download raw body.
tog is sluggish with FreeBSD src.git
On Fri, Dec 25, 2020 at 08:40:56PM +0100, Christian Weisgerber wrote:
> Stefan Sperling:
>
> > 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.
>
> Indeed, that is a noticeable improvement.
>
> I had wondered about passing idmap from the log to the diff view,
> but got_reflist_object_id_map_create() takes 6 ms for FreeBSD src.git
> on my test box, so I abandoned that thought.
>
> > 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.
>
> Why copy and free it for each view? How about keeping a global ref
> list?
Like in the patch below?
This patch improves performance of a blame view -> diff view switch.
> > We could even optimize
> > got_ref_list() itself if this start-up overhead is not acceptable.
>
> There _is_ a noticeable start-up overhead, but I don't know what's
> causing it.
Probably a combination of things.
Another change we may want is optional skipping of got_ref_list() inside
got_repo_match_object_id(). It needs refs to match user input with tag names.
diff refs/heads/main refs/heads/togref
blob - a28a00571d687d94f4f9215df59a0fe82423e7c9
blob + ee2324cc37e9714754f8a581d0e546d0ef5f385c
--- tog/tog.c
+++ tog/tog.c
@@ -125,7 +125,33 @@ struct tog_color {
};
SIMPLEQ_HEAD(tog_colors, tog_color);
+static struct got_reflist_head tog_refs = SIMPLEQ_HEAD_INITIALIZER(tog_refs);
+static struct got_reflist_object_id_map *tog_refs_idmap;
+
static const struct got_error *
+tog_load_refs(struct got_repository *repo)
+{
+ const struct got_error *err;
+
+ err = got_ref_list(&tog_refs, repo, NULL, got_ref_cmp_by_name, NULL);
+ if (err)
+ return err;
+
+ return got_reflist_object_id_map_create(&tog_refs_idmap, &tog_refs,
+ repo);
+}
+
+static void
+tog_free_refs(void)
+{
+ if (tog_refs_idmap) {
+ got_reflist_object_map_free(tog_refs_idmap);
+ tog_refs_idmap = NULL;
+ }
+ got_ref_list_free(&tog_refs);
+}
+
+static const struct got_error *
add_color(struct tog_colors *colors, const char *pattern,
int idx, short color)
{
@@ -262,8 +288,6 @@ struct tog_diff_view_state {
int ignore_whitespace;
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,8 +328,6 @@ struct tog_log_view_state {
char *head_ref_name;
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;
@@ -412,7 +434,6 @@ struct tog_reflist_entry {
TAILQ_HEAD(tog_reflist_head, tog_reflist_entry);
struct tog_ref_view_state {
- struct got_reflist_head simplerefs; /* SIMPLEQ */
struct tog_reflist_head refs; /* TAILQ */
struct tog_reflist_entry *first_displayed_entry;
struct tog_reflist_entry *last_displayed_entry;
@@ -1578,7 +1599,7 @@ draw_commits(struct tog_view *view)
err = got_object_id_str(&id_str, s->selected_entry->id);
if (err)
return err;
- refs = got_reflist_object_id_map_lookup(s->idmap,
+ refs = got_reflist_object_id_map_lookup(tog_refs_idmap,
s->selected_entry->id);
if (refs) {
err = build_refs_str(&refs_str, refs,
@@ -2125,11 +2146,6 @@ 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;
}
@@ -2258,8 +2274,6 @@ open_log_view(struct tog_view *view, struct got_object
struct got_commit_graph *thread_graph = NULL;
int errcode;
- SIMPLEQ_INIT(&s->refs);
-
if (in_repo_path != s->in_repo_path) {
free(s->in_repo_path);
s->in_repo_path = strdup(in_repo_path);
@@ -2271,13 +2285,6 @@ open_log_view(struct tog_view *view, struct got_object
TAILQ_INIT(&s->commits.head);
s->commits.ncommits = 0;
- 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) {
s->head_ref_name = strdup(head_ref_name);
@@ -2543,19 +2550,10 @@ 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);
+ tog_free_refs();
+ err = tog_load_refs(s->repo);
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)
@@ -2758,6 +2756,10 @@ cmd_log(int argc, char *argv[])
if (error)
goto done;
+ error = tog_load_refs(repo);
+ if (error)
+ goto done;
+
if (start_commit == NULL) {
error = got_repo_match_object_id(&start_id, &label,
worktree ? got_worktree_get_head_ref_name(worktree) :
@@ -2804,6 +2806,7 @@ done:
got_repo_close(repo);
if (worktree)
got_worktree_close(worktree);
+ tog_free_refs();
return error;
}
@@ -3281,7 +3284,7 @@ create_diff(struct tog_diff_view_state *s)
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);
+ refs = got_reflist_object_id_map_lookup(tog_refs_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,
@@ -3414,8 +3417,6 @@ open_diff_view(struct tog_view *view, struct got_objec
const struct got_error *err;
struct tog_diff_view_state *s = &view->state.diff;
- SIMPLEQ_INIT(&s->refs);
-
if (id1 != NULL && id2 != NULL) {
int type1, type2;
err = got_object_get_type(&type1, repo, id1);
@@ -3507,26 +3508,6 @@ 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 (err) {
- free(s->id1);
- s->id1 = NULL;
- free(s->id2);
- s->id2 = NULL;
- 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 */
diff_view_indicate_progress(view);
@@ -3540,9 +3521,6 @@ open_diff_view(struct tog_view *view, struct got_objec
free(s->id2);
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;
}
@@ -3571,11 +3549,6 @@ 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;
}
@@ -3855,6 +3828,10 @@ cmd_diff(int argc, char *argv[])
if (error)
goto done;
+ error = tog_load_refs(repo);
+ if (error)
+ goto done;
+
error = got_repo_match_object_id(&id1, &label1, id_str1,
GOT_OBJ_TYPE_ANY, 1, repo);
if (error)
@@ -3884,6 +3861,7 @@ done:
got_repo_close(repo);
if (worktree)
got_worktree_close(worktree);
+ tog_free_refs();
return error;
}
@@ -4748,6 +4726,10 @@ cmd_blame(int argc, char *argv[])
if (error)
goto done;
+ error = tog_load_refs(repo);
+ if (error)
+ goto done;
+
if (commit_id_str == NULL) {
struct got_reference *head_ref;
error = got_ref_open(&head_ref, repo, worktree ?
@@ -4794,6 +4776,7 @@ done:
got_worktree_close(worktree);
if (repo)
got_repo_close(repo);
+ tog_free_refs();
return error;
}
@@ -5558,6 +5541,10 @@ cmd_tree(int argc, char *argv[])
if (error)
goto done;
+ error = tog_load_refs(repo);
+ if (error)
+ goto done;
+
if (commit_id_arg == NULL) {
error = got_repo_match_object_id(&commit_id, &label,
worktree ? got_worktree_get_head_ref_name(worktree) :
@@ -5620,23 +5607,18 @@ done:
got_object_tree_close(tree);
if (repo)
got_repo_close(repo);
+ tog_free_refs();
return error;
}
static const struct got_error *
ref_view_load_refs(struct tog_ref_view_state *s)
{
- const struct got_error *err;
struct got_reflist_entry *sre;
struct tog_reflist_entry *re;
- err = got_ref_list(&s->simplerefs, s->repo, NULL,
- got_ref_cmp_by_name, NULL);
- if (err)
- return err;
-
s->nrefs = 0;
- SIMPLEQ_FOREACH(sre, &s->simplerefs, entry) {
+ SIMPLEQ_FOREACH(sre, &tog_refs, entry) {
if (strncmp(got_ref_get_name(sre->ref), "refs/got/", 9) == 0)
continue;
@@ -5644,11 +5626,14 @@ ref_view_load_refs(struct tog_ref_view_state *s)
if (re == NULL)
return got_error_from_errno("malloc");
- re->ref = sre->ref;
+ re->ref = got_ref_dup(sre->ref);
+ if (re->ref == NULL)
+ return got_error_from_errno("got_ref_dup");
re->idx = s->nrefs++;
TAILQ_INSERT_TAIL(&s->refs, re, entry);
}
+ s->first_displayed_entry = TAILQ_FIRST(&s->refs);
return NULL;
}
@@ -5660,9 +5645,9 @@ ref_view_free_refs(struct tog_ref_view_state *s)
while (!TAILQ_EMPTY(&s->refs)) {
re = TAILQ_FIRST(&s->refs);
TAILQ_REMOVE(&s->refs, re, entry);
+ got_ref_close(re->ref);
free(re);
}
- got_ref_list_free(&s->simplerefs);
}
static const struct got_error *
@@ -5674,7 +5659,6 @@ open_ref_view(struct tog_view *view, struct got_reposi
s->selected_entry = 0;
s->repo = repo;
- SIMPLEQ_INIT(&s->simplerefs);
TAILQ_INIT(&s->refs);
SIMPLEQ_INIT(&s->colors);
@@ -5682,8 +5666,6 @@ open_ref_view(struct tog_view *view, struct got_reposi
if (err)
return err;
- s->first_displayed_entry = TAILQ_FIRST(&s->refs);
-
if (has_colors() && getenv("TOG_COLORS") != NULL) {
err = add_color(&s->colors, "^refs/heads/",
TOG_COLOR_REFS_HEADS,
@@ -6191,6 +6173,10 @@ input_ref_view(struct tog_view **new_view, struct tog_
ref_scroll_down(s, view->nlines - 1);
break;
case CTRL('l'):
+ tog_free_refs();
+ err = tog_load_refs(s->repo);
+ if (err)
+ break;
ref_view_free_refs(s);
err = ref_view_load_refs(s);
break;
@@ -6272,6 +6258,10 @@ cmd_ref(int argc, char *argv[])
if (error)
goto done;
+ error = tog_load_refs(repo);
+ if (error)
+ goto done;
+
view = view_open(0, 0, 0, 0, TOG_VIEW_REF);
if (view == NULL) {
error = got_error_from_errno("view_open");
@@ -6293,6 +6283,7 @@ done:
free(cwd);
if (repo)
got_repo_close(repo);
+ tog_free_refs();
return error;
}
tog is sluggish with FreeBSD src.git