"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 17:58:07 +1000

Download raw body.

Thread
  • Stefan Sperling:

    tog: basic diff regress

  • 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.
    
    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)
    @@ -1933,7 +1932,8 @@ tog_io_close(void)
     static const struct got_error *
     tog_io_close(void)
     {
    -	const struct got_error *err = NULL;
    +	const struct got_error	*err = NULL;
    +	int			 rc;
     
     	if (tog_io.cin && fclose(tog_io.cin) == EOF)
     		err = got_ferror(tog_io.cin, GOT_ERR_IO);
    @@ -1942,6 +1942,10 @@ tog_io_close(void)
     	if (tog_io.f && fclose(tog_io.f) == EOF && err == NULL)
     		err = got_ferror(tog_io.f, GOT_ERR_IO);
     
    +	rc = pthread_cond_destroy(&tog_io.blame_is_drawn);
    +	if (rc && err == NULL)
    +		err = got_error_set_errno(rc, "pthread_cond_destroy");
    +
     	return err;
     }
     
    @@ -4177,6 +4181,7 @@ init_mock_term(const char *test_script_path)
     init_mock_term(const char *test_script_path)
     {
     	const struct got_error	*err = NULL;
    +	int			 rc;
     
     	if (test_script_path == NULL || *test_script_path == '\0')
     		return got_error_msg(GOT_ERR_IO, "TOG_TEST_SCRIPT not defined");
    @@ -4211,7 +4216,14 @@ init_mock_term(const char *test_script_path)
     		err = got_error_msg(GOT_ERR_IO,
     		    "newterm: failed to initialise curses");
     
    +	rc = pthread_cond_init(&tog_io.blame_is_drawn, NULL);
    +	if (rc) {
    +		err = got_error_set_errno(rc, "pthread_cond_init");
    +		goto done;
    +	}
    +
     	using_mock_io = 1;
    +
     done:
     	if (err)
     		tog_io_close();
    @@ -6002,6 +6014,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 +6144,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 +6241,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");
    
    -- 
    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