Skip to content

Commit

Permalink
Slightly rework pipeline threading
Browse files Browse the repository at this point in the history 
1. fix a typo in preview pipe loop that waited on a flag belonging to main pipe
2. manage the "processing" state in the top-most function, around the loop
3. use inner and outer thread-lock perf timings
  • Loading branch information
@aurelienpierre
aurelienpierre committed Jan 15, 2025
1 parent 65e7e1d commit f1e14b4
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 24 deletions.
55 changes: 37 additions & 18 deletions src/develop/develop.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,7 @@ void dt_dev_refresh_ui_images_real(dt_develop_t *dev)
// Benefit is atomics are de-facto thread-safe.
if(dt_atomic_get_int(&dev->preview_pipe->shutdown) && dev->preview_pipe->status != DT_DEV_PIXELPIPE_VALID)
{
if(!dev->preview_pipe->processing)
dt_dev_process_preview(dev);
if(!dev->preview_pipe->processing) dt_dev_process_preview(dev);
// else : join current pipe
}
// When entering darkroom, the GUI will size itself and call the
Expand All @@ -248,8 +247,7 @@ void dt_dev_refresh_ui_images_real(dt_develop_t *dev)
// the main preview pipe.
if(dt_atomic_get_int(&dev->pipe->shutdown) && dev->pipe->status != DT_DEV_PIXELPIPE_VALID)
{
if(!dev->pipe->processing)
dt_dev_process_image(dev);
if(!dev->pipe->processing) dt_dev_process_image(dev);
// else : join current pipe
}
}
Expand Down Expand Up @@ -350,9 +348,10 @@ void dt_dev_invalidate_all_real(dt_develop_t *dev)

void dt_dev_process_preview_job(dt_develop_t *dev)
{
dt_pthread_mutex_lock(&dev->preview_pipe->busy_mutex);

gboolean finish_on_error = FALSE;

dt_pthread_mutex_lock(&dev->preview_pipe->busy_mutex);
dt_control_log_busy_enter();
dt_control_toast_busy_enter();

Expand All @@ -366,27 +365,36 @@ void dt_dev_process_preview_job(dt_develop_t *dev)
if(!finish_on_error)
dt_dev_pixelpipe_set_input(dev->preview_pipe, dev, (float *)buf.buf, buf.width, buf.height, buf.iscale);

while(dev->pipe->status == DT_DEV_PIXELPIPE_DIRTY && !finish_on_error)
dev->preview_pipe->processing = 1;
while(dev->preview_pipe->status == DT_DEV_PIXELPIPE_DIRTY && !finish_on_error)
{
dt_times_t thread_start;
dt_get_times(&thread_start);

// adjust pipeline according to changed flag set by {add,pop}_history_item.
// this locks dev->history_mutex.
dt_times_t start;
dt_get_times(&start);

// adjust pipeline according to changed flag set by {add,pop}_history_item.
// this locks dev->history_mutex.
dt_dev_pixelpipe_change(dev->preview_pipe, dev);

dt_pthread_mutex_lock(&dev->pipe_mutex);

dt_times_t start;
dt_get_times(&start);

int ret = dt_dev_pixelpipe_process(dev->preview_pipe, dev, 0, 0, dev->preview_pipe->processed_width,
dev->preview_pipe->processed_height, 1.f);

dt_show_times(&start, "[dev_process_preview] pixel pipeline processing");

dt_pthread_mutex_unlock(&dev->pipe_mutex);

if(ret && dev->preview_pipe->status == DT_DEV_PIXELPIPE_INVALID) finish_on_error = TRUE;
dt_show_times(&thread_start, "[dev_process_preview] pixel pipeline thread");
dt_dev_average_delay_update(&thread_start, &dev->preview_average_delay);

dt_show_times(&start, "[dev_process_preview] pixel pipeline processing");
dt_dev_average_delay_update(&start, &dev->preview_average_delay);
if(ret && dev->preview_pipe->status == DT_DEV_PIXELPIPE_INVALID) finish_on_error = TRUE;
}
dev->preview_pipe->processing = 0;

dt_control_log_busy_leave();
dt_control_toast_busy_leave();
Expand All @@ -412,6 +420,7 @@ void dt_dev_process_image_job(dt_develop_t *dev)
gboolean finish_on_error = FALSE;

dt_pthread_mutex_lock(&dev->pipe->busy_mutex);

dt_control_log_busy_enter();
dt_control_toast_busy_enter();
dt_mipmap_buffer_t buf;
Expand All @@ -423,10 +432,11 @@ void dt_dev_process_image_job(dt_develop_t *dev)
dt_dev_pixelpipe_set_input(dev->pipe, dev, (float *)buf.buf, buf.width, buf.height, 1.0);

float scale = 1.f, zoom_x = 1.f, zoom_y = 1.f;
dev->pipe->processing = 1;
while(dev->pipe->status == DT_DEV_PIXELPIPE_DIRTY && !finish_on_error)
{
dt_times_t start;
dt_get_times(&start);
dt_times_t thread_start;
dt_get_times(&thread_start);

// adjust pipeline according to changed flag set by {add,pop}_history_item.
// dt_dev_pixelpipe_change() will clear the changed value
Expand Down Expand Up @@ -463,14 +473,23 @@ void dt_dev_process_image_job(dt_develop_t *dev)
int y = MAX(0, scale * dev->pipe->processed_height * (.5 + zoom_y) - ht / 2);

dt_pthread_mutex_lock(&dev->pipe_mutex);

dt_times_t start;
dt_get_times(&start);

int ret = dt_dev_pixelpipe_process(dev->pipe, dev, x, y, wd, ht, scale);

dt_show_times(&start, "[dev_process_image] pixel pipeline processing");

dt_pthread_mutex_unlock(&dev->pipe_mutex);

if(ret && dev->pipe->status == DT_DEV_PIXELPIPE_INVALID) finish_on_error = TRUE;
dt_show_times(&thread_start, "[dev_process_image] pixel pipeline thread");

dt_show_times(&start, "[dev_process_image] pixel pipeline processing");
dt_dev_average_delay_update(&start, &dev->average_delay);
dt_dev_average_delay_update(&thread_start, &dev->average_delay);

if(ret && dev->pipe->status == DT_DEV_PIXELPIPE_INVALID) finish_on_error = TRUE;
}
dev->pipe->processing = 0;

// cool, we got a new image!
if(!finish_on_error)
Expand Down Expand Up @@ -850,7 +869,7 @@ const dt_dev_history_item_t *dt_dev_get_history_item(dt_develop_t *dev, struct d
return NULL;
}

#define AUTO_SAVE_TIMEOUT 10000
#define AUTO_SAVE_TIMEOUT 30000

static int _auto_save_edit(gpointer data)
{
Expand Down
8 changes: 2 additions & 6 deletions src/develop/pixelpipe_hb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,6 @@ void dt_dev_pixelpipe_change(dt_dev_pixelpipe_t *pipe, struct dt_develop_t *dev)
dt_get_times(&start);

dt_pthread_mutex_lock(&dev->history_mutex);
dt_atomic_set_int(&pipe->shutdown, FALSE);

// mask display off as a starting point
pipe->mask_display = DT_DEV_PIXELPIPE_DISPLAY_NONE;
Expand Down Expand Up @@ -636,6 +635,7 @@ void dt_dev_pixelpipe_change(dt_dev_pixelpipe_t *pipe, struct dt_develop_t *dev)

dt_pthread_mutex_unlock(&dev->history_mutex);

dt_atomic_set_int(&pipe->shutdown, FALSE);
dt_show_times(&start, "[dt_dev_pixelpipe_change] pipeline resync on the current modules stack");
}

Expand Down Expand Up @@ -2294,7 +2294,6 @@ static int dt_dev_pixelpipe_process_rec_and_backcopy(dt_dev_pixelpipe_t *pipe, d
#define KILL_SWITCH_PIPE \
if(dt_atomic_get_int(&pipe->shutdown)) \
{ \
pipe->processing = 0; \
pipe->status = DT_DEV_PIXELPIPE_DIRTY; \
return 1; \
}
Expand All @@ -2304,7 +2303,6 @@ int dt_dev_pixelpipe_process(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev, int x,
{
KILL_SWITCH_PIPE

pipe->processing = 1;
pipe->backbuf = NULL;
pipe->opencl_enabled = dt_opencl_update_settings(); // update enabled flag and profile from preferences
pipe->devid = (pipe->opencl_enabled) ? dt_opencl_lock_device(pipe->type)
Expand Down Expand Up @@ -2398,7 +2396,7 @@ restart:;
}

// release resources:
if (pipe->forms)
if(pipe->forms)
{
g_list_free_full(pipe->forms, (void (*)(void *))dt_masks_free_form);
pipe->forms = NULL;
Expand All @@ -2411,7 +2409,6 @@ restart:;
// ... and in case of other errors ...
if(err)
{
pipe->processing = 0;
pipe->status = DT_DEV_PIXELPIPE_INVALID;
return 1;
}
Expand Down Expand Up @@ -2443,7 +2440,6 @@ restart:;

// printf("pixelpipe homebrew process end\n");
pipe->status = DT_DEV_PIXELPIPE_VALID;
pipe->processing = 0;
return 0;
}

Expand Down

0 comments on commit f1e14b4

Please sign in to comment.