Download raw body.
show base commit markers in tog log view
On Mon, Jul 24, 2023 at 12:37:44AM +0200, Stefan Sperling wrote:
> On Fri, Jul 21, 2023 at 03:14:06PM +0200, Stefan Sperling wrote:
> > A problem that applies to both this patch and got branch -l:
> > A mixed-commit work tree will be displayed as up-to-date since the current
> > logic only considers the work tree's global base commit.
> > Could we iterate over the file index and check whether any file is based
> > on a different commit, and if so show the out-of-date symbol ~?
>
> The above suggestion I made results in a performance regression,
> as Omar pointed out on IRC:
>
> 20:47 <@op2> one consequence of the new work tree base commit marker
> in tog is that it is slower to open in src.git :/
> 20:48 <@op2> xterm stays blank for ~2 seconds before it start listing
> commits, while in 0.90 it was istantaneous
>
> The patch below attempts to fix this regression by letting the
> log thread walk the file index once it has fetched the initial
> batch of commits required to fill the display. In my testing this
> lets tog start up as fast at it did before and already accept input
> before the marker appears in a large work tree.
> The delay is still noticable in some cases, e.g. when pressing page-down
> right after starting tog, the display will only scroll further once the
> commit marker has been drawn. But this is a small one-time lag and I could
> live with it.
>
> Regarding 'got branch -l' I am podering moving mixed-commit detection
> to 'got status' since this command is already walking the file index.
> Or will we accept branch -l being slower now?
The previous diff made tog hang forever when 'q' or Ctrl-C were used to
quit tog before the base commit marker was drawn. Fixed in this version
by adding a missing check of the *a->quit flag. Without this check the log
thread goes to sleep after getting the work tree's state, waiting for more
commits to fetch. But it will never be woken up again since the main thread
did already signal 'stop' to the log thread with commits_needed == 0 and
a->quit == 1.
-----------------------------------------------
commit 2d9a8424e0be050d91aa43c21b4cbafba2b21d51 (togmarker)
from: Stefan Sperling <stsp@stsp.name>
date: Mon Jul 24 08:08:30 2023 UTC
load tog's worktree base commit marker in the log thread for startup speed
diff ccdddf69864eb9ac5c10c0f5d58b68df1cf26ea1 2d9a8424e0be050d91aa43c21b4cbafba2b21d51
commit - ccdddf69864eb9ac5c10c0f5d58b68df1cf26ea1
commit + 2d9a8424e0be050d91aa43c21b4cbafba2b21d51
blob - 07cb22fd98f7b10c110a8a5d0c0627770c5b7e76
blob + 02e203f9626eba60a84658a1f9e7a8d0fb043418
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -137,13 +137,14 @@ const struct got_error *got_worktree_set_base_commit_i
* Get the state of the work tree. If the work tree's global base commit is
* the tip of the work tree's current branch, and each file in the index is
* based on this same commit, the char out parameter will be
- * GOT_WORKTREE_UPTODATE, else it will be GOT_WORKTREE_OUTOFDATE.
+ * GOT_WORKTREE_STATE_UPTODATE, else it will be GOT_WORKTREE_STATE_OUTOFDATE.
*/
const struct got_error *got_worktree_get_state(char *,
struct got_repository *, struct got_worktree *);
-#define GOT_WORKTREE_UPTODATE '*'
-#define GOT_WORKTREE_OUTOFDATE '~'
+#define GOT_WORKTREE_STATE_UNKNOWN ' '
+#define GOT_WORKTREE_STATE_UPTODATE '*'
+#define GOT_WORKTREE_STATE_OUTOFDATE '~'
/*
* Obtain a parsed representation of this worktree's got.conf file.
blob - 6b14bc178bf5fbc6065088656b0ecae4a8fd2aad
blob + e0c7318204bffd754947e3848dcb9e7b6ad3ae9e
--- lib/worktree.c
+++ lib/worktree.c
@@ -3273,7 +3273,7 @@ got_worktree_get_state(char *state, struct got_reposit
if (err)
goto done;
- *state = GOT_WORKTREE_OUTOFDATE;
+ *state = GOT_WORKTREE_STATE_UNKNOWN;
base_id = got_worktree_get_base_commit_id(worktree);
if (got_object_id_cmp(base_id, head_id) == 0) {
@@ -3284,10 +3284,13 @@ got_worktree_get_state(char *state, struct got_reposit
err = got_fileindex_for_each_entry_safe(fileindex,
check_mixed_commits, worktree);
if (err == NULL)
- *state = GOT_WORKTREE_UPTODATE;
- else if (err->code == GOT_ERR_MIXED_COMMITS)
+ *state = GOT_WORKTREE_STATE_UPTODATE;
+ else if (err->code == GOT_ERR_MIXED_COMMITS) {
+ *state = GOT_WORKTREE_STATE_OUTOFDATE;
err = NULL;
- }
+ }
+ } else
+ *state = GOT_WORKTREE_STATE_OUTOFDATE;
done:
free(head_id);
blob - 897b0c14098c0b8440dbdfbedd2bb8c0792e6efb
blob + 7460e9344c4e5b28dd17ca3a1d586cb14e0a5a2d
--- tog/tog.c
+++ tog/tog.c
@@ -380,6 +380,8 @@ struct tog_log_thread_args {
int limit_match;
regex_t *limit_regex;
struct commit_queue *limit_commits;
+ struct got_worktree *worktree;
+ int need_commit_marker;
};
struct tog_log_view_state {
@@ -730,7 +732,7 @@ static const struct got_error *open_log_view(struct to
static const struct got_error *open_log_view(struct tog_view *,
struct got_object_id *, struct got_repository *,
- const char *, const char *, int);
+ const char *, const char *, int, struct got_worktree *);
static const struct got_error * show_log_view(struct tog_view *);
static const struct got_error *input_log_view(struct tog_view **,
struct tog_view *, int);
@@ -2423,6 +2425,10 @@ draw_commit(struct tog_view *view, struct commit_queue
struct tog_color *tc;
struct got_reflist_head *refs;
+ if (tog_base_commit.id != NULL && tog_base_commit.idx == -1 &&
+ got_object_id_cmp(id, tog_base_commit.id) == 0)
+ tog_base_commit.idx = entry->idx;
+
committer_time = got_object_commit_get_committer_time(commit);
if (gmtime_r(&committer_time, &tm) == NULL)
return got_error_from_errno("gmtime_r");
@@ -2482,7 +2488,7 @@ draw_commit(struct tog_view *view, struct commit_queue
waddwstr(view->window, wauthor);
col += author_width;
while (col < avail && author_width < author_display_cols + 2) {
- if (tog_base_commit.id != NULL &&
+ if (tog_base_commit.marker != GOT_WORKTREE_STATE_UNKNOWN &&
author_width == marker_column &&
entry->idx == tog_base_commit.idx) {
tc = get_color(&s->colors, TOG_COLOR_COMMIT);
@@ -2709,10 +2715,6 @@ queue_commits(struct tog_log_thread_args *a)
TAILQ_INSERT_TAIL(&a->real_commits->head, entry, entry);
a->real_commits->ncommits++;
- if (tog_base_commit.id != NULL && tog_base_commit.idx == -1 &&
- got_object_id_cmp(&id, tog_base_commit.id) == 0)
- tog_base_commit.idx = entry->idx;
-
if (*a->limiting) {
err = match_commit(&limit_match, &id, commit,
a->limit_regex);
@@ -3360,6 +3362,37 @@ log_thread(void *arg)
goto done;
}
+ if (a->commits_needed == 0 &&
+ a->need_commit_marker && a->worktree) {
+ errcode = pthread_mutex_unlock(&tog_mutex);
+ if (errcode) {
+ err = got_error_set_errno(errcode,
+ "pthread_mutex_unlock");
+ goto done;
+ }
+ err = got_worktree_get_state(
+ &tog_base_commit.marker,
+ a->repo, a->worktree);
+ if (err)
+ goto done;
+ errcode = pthread_mutex_lock(&tog_mutex);
+ if (errcode) {
+ err = got_error_set_errno(errcode,
+ "pthread_mutex_lock");
+ goto done;
+ }
+ a->need_commit_marker = 0;
+ /*
+ * The main thread did not close this
+ * work tree yet. Close it now.
+ */
+ got_worktree_close(a->worktree);
+ a->worktree = NULL;
+
+ if (*a->quit)
+ done = 1;
+ }
+
if (done)
a->commits_needed = 0;
else {
@@ -3385,6 +3418,10 @@ done:
if (err) {
tog_thread_error = 1;
pthread_cond_signal(&a->commit_loaded);
+ if (a->worktree) {
+ got_worktree_close(a->worktree);
+ a->worktree = NULL;
+ }
}
return (void *)err;
}
@@ -3705,7 +3742,8 @@ open_log_view(struct tog_view *view, struct got_object
static const struct got_error *
open_log_view(struct tog_view *view, struct got_object_id *start_id,
struct got_repository *repo, const char *head_ref_name,
- const char *in_repo_path, int log_branches)
+ const char *in_repo_path, int log_branches,
+ struct got_worktree *worktree)
{
const struct got_error *err = NULL;
struct tog_log_view_state *s = &view->state.log;
@@ -3820,6 +3858,9 @@ done:
s->thread_args.limiting = &s->limit_view;
s->thread_args.limit_regex = &s->limit_regex;
s->thread_args.limit_commits = &s->limit_commits;
+ s->thread_args.worktree = worktree;
+ if (worktree)
+ s->thread_args.need_commit_marker = 1;
done:
if (err) {
if (view->close == NULL)
@@ -4394,9 +4435,8 @@ set_tog_base_commit(struct got_repository *repo, struc
return got_error_from_errno( "got_object_id_dup");
tog_base_commit.idx = -1;
-
- return got_worktree_get_state(&tog_base_commit.marker, repo,
- worktree);
+ tog_base_commit.marker = GOT_WORKTREE_STATE_UNKNOWN;
+ return NULL;
}
static const struct got_error *
@@ -4551,18 +4591,20 @@ cmd_log(int argc, char *argv[])
error = got_error_from_errno("view_open");
goto done;
}
+
+ if (worktree) {
+ error = set_tog_base_commit(repo, worktree);
+ if (error != NULL)
+ goto done;
+ }
+
error = open_log_view(view, start_id, repo, head_ref_name,
- in_repo_path, log_branches);
+ in_repo_path, log_branches, worktree);
if (error)
goto done;
if (worktree) {
- error = set_tog_base_commit(repo, worktree);
- if (error != NULL)
- goto done;
-
- /* Release work tree lock. */
- got_worktree_close(worktree);
+ /* The work tree will be closed by the log thread. */
worktree = NULL;
}
@@ -6814,7 +6856,7 @@ log_annotated_line(struct tog_view **new_view, int beg
if (log_view == NULL)
return got_error_from_errno("view_open");
- err = open_log_view(log_view, id, repo, GOT_REF_HEAD, "", 0);
+ err = open_log_view(log_view, id, repo, GOT_REF_HEAD, "", 0, NULL);
if (err)
view_close(log_view);
else
@@ -7616,7 +7658,7 @@ log_selected_tree_entry(struct tog_view **new_view, in
return err;
err = open_log_view(log_view, s->commit_id, s->repo, s->head_ref_name,
- path, 0);
+ path, 0, NULL);
if (err)
view_close(log_view);
else
@@ -8474,7 +8516,7 @@ log_ref_entry(struct tog_view **new_view, int begin_y,
}
err = open_log_view(log_view, commit_id, repo,
- got_ref_get_name(re->ref), "", 0);
+ got_ref_get_name(re->ref), "", 0, NULL);
done:
if (err)
view_close(log_view);
show base commit markers in tog log view