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

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: tog: basic diff regress
To:
Game of Trees <gameoftrees@openbsd.org>
Date:
Sun, 16 Apr 2023 19:03:00 +1000

Download raw body.

Thread
  • Stefan Sperling:

    tog: basic diff regress

  • On 23-04-16 05:58PM, Mark Jamsek wrote:
    > On 23-04-15 05:23PM, Stefan Sperling wrote:
    > > On Sat, Apr 15, 2023 at 09:46:51PM +1000, Mark Jamsek wrote:
    > > > The below diff (which applies on top of main) adds a new WAIT_FOR_UI
    > > > instruction to the test harness, and a basic blame regress test that
    > > > demonstrates its use.  When the instruction is received a flag is set,
    > > > which delays processing further instructions till it is unset, which, in
    > > > this case, is when the blame process is complete. This can easily be
    > > > extrapolated to future cases that require waiting till the UI has been
    > > > drawn.
    > > 
    > > Looks fine. ok
    > > 
    > > I would have expected pthread_cond_wait() to be involved while waiting.
    > > This would require more than one WAIT-style instruction to identify
    > > which condition the main thread should for.
    > > I guess the way you implemented this we will essentially busy-loop
    > > in the view input handlers until the variable is flipped by the other
    > > thread? That is a bit less elegant than waiting via pthread APIs.
    > > But of course if it works it can be good enough for tests. And in
    > > any case it is better than sleeping for some fixed amount of time.
    > 
    > Sure, that would be better than busy waiting the view loop! And if we
    > keep the wait_for_ui flag, we can actually keep the general WAIT_FOR_UI
    > instruction and wait conditions will just be context dependent. So we
    > can use WAIT_FOR_UI irrespective of the view.
    
    Please disregard the previous diff:
    
    diff /home/mark/src/got
    commit - c736b84ab8efb53399d58afe57a2e40c4c7dd1b5
    path + /home/mark/src/got
    blob - 8c21271a99fe7416ffc1a6b97ce8ce11403a6f35
    file + tog/tog.c
    --- tog/tog.c
    +++ tog/tog.c
    @@ -614,10 +614,11 @@ struct tog_io {
     
     /* curses io for tog regress */
     struct tog_io {
    -	FILE	*cin;
    -	FILE	*cout;
    -	FILE	*f;
    -	int	 wait_for_ui;
    +	FILE		*cin;
    +	FILE		*cout;
    +	FILE		*f;
    +	int		 wait_for_ui;
    +	pthread_cond_t	 blame_is_drawn;
     } tog_io;
     static int using_mock_io;
     
    @@ -1632,9 +1633,6 @@ tog_read_script_key(FILE *script, int *ch, int *done)
     
     	*ch = -1;
     
    -	if (tog_io.wait_for_ui)
    -		goto done;
    -
     	if (getline(&line, &linesz, script) == -1) {
     		if (feof(script)) {
     			*done = 1;
    @@ -1644,6 +1642,7 @@ tog_read_script_key(FILE *script, int *ch, int *done)
     			goto done;
     		}
     	}
    +
     	if (strncasecmp(line, "WAIT_FOR_UI", 11) == 0)
     		tog_io.wait_for_ui = 1;
     	else if (strncasecmp(line, "KEY_ENTER", 9) == 0)
    @@ -4212,6 +4211,7 @@ done:
     		    "newterm: failed to initialise curses");
     
     	using_mock_io = 1;
    +
     done:
     	if (err)
     		tog_io_close();
    @@ -6002,6 +6002,15 @@ draw_blame(struct tog_view *view)
     	if (view->gline > blame->nlines)
     		view->gline = blame->nlines;
     
    +	if (tog_io.wait_for_ui) {
    +		int rc;
    +
    +		rc = pthread_cond_wait(&tog_io.blame_is_drawn, &tog_mutex);
    +		if (rc)
    +			return got_error_set_errno(rc, "pthread_cond_wait");
    +		tog_io.wait_for_ui = 0;
    +	}
    +
     	if (asprintf(&line, "[%d/%d] %s%s", view->gline ? view->gline :
     	    s->first_displayed_line - 1 + s->selected_line, blame->nlines,
     	    s->blame_complete ? "" : "annotating... ", s->path) == -1) {
    @@ -6123,11 +6132,6 @@ draw_blame(struct tog_view *view)
     
     	view_border(view);
     
    -	if (tog_io.wait_for_ui) {
    -		if (s->blame_complete)
    -			tog_io.wait_for_ui = 0;
    -	}
    -
     	return NULL;
     }
     
    @@ -6225,6 +6229,14 @@ blame_thread(void *arg)
     	ta->repo = NULL;
     	*ta->complete = 1;
     
    +	if (tog_io.wait_for_ui) {
    +		int rc;
    +
    +		rc = pthread_cond_signal(&tog_io.blame_is_drawn);
    +		if (rc && err == NULL)
    +			err = got_error_set_errno(rc, "pthread_cond_signal");
    +	}
    +
     	errcode = pthread_mutex_unlock(&tog_mutex);
     	if (errcode && err == NULL)
     		err = got_error_set_errno(errcode, "pthread_mutex_unlock");
    @@ -6505,6 +6517,14 @@ open_blame_view(struct tog_view *view, char *path,
     	view->search_setup = search_setup_blame_view;
     	view->search_next = search_next_view_match;
     
    +	if (using_mock_io) {
    +		int rc;
    +
    +		rc = pthread_cond_init(&tog_io.blame_is_drawn, NULL);
    +		if (rc)
    +			return got_error_set_errno(rc, "pthread_cond_init");
    +	}
    +
     	return run_blame(view);
     }
     
    @@ -6524,6 +6544,14 @@ close_blame_view(struct tog_view *view)
     		got_object_qid_free(blamed_commit);
     	}
     
    +	if (using_mock_io) {
    +		int rc;
    +
    +		rc = pthread_cond_destroy(&tog_io.blame_is_drawn);
    +		if (rc && err == NULL)
    +			err = got_error_set_errno(rc, "pthread_cond_destroy");
    +	}
    +
     	free(s->path);
     	free_colors(&s->colors);
     	return err;
    
    -- 
    Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
    GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68
    
  • Stefan Sperling:

    tog: basic diff regress