Skip to content

Commit

Permalink
pipeline cache : simplify the code, fix inconsistent states ?
Browse files Browse the repository at this point in the history 
The pipe cache code was a tangled mess taking input from GUI (active module) and pipeline. That's prone to errors. Plus it had a couple of smart-ass perks (ROI-independent cache ? what for ?).

Changes made : 
- each module gets a global hash representing its internal params, blending params, previous modules and ROI size.
- this hash is attributed at pipeline (re)construction and independant from GUI events. It gets recomputed on the whole pipeline.
- this hash is used as a key to fetch cache lines.

Any upstream change in a module will automatically be propagated to the next hashes. Therefore, we only need to find the latest valid hash (starting from the end of the pipeline) and start recomputing from there. No need to track what module the user was tingling when recomputation was requested.
  • Loading branch information
@aurelienpierre
aurelienpierre committed Dec 1, 2023
1 parent a3b311b commit a95375f
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 157 deletions.
7 changes: 1 addition & 6 deletions src/develop/imageop.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,7 @@ void dt_iop_commit_params(dt_iop_module_t *module, dt_iop_params_t *params,
// 2. compute the hash only if piece is enabled

piece->hash = 0;
piece->global_hash = 0;

if(piece->enabled)
{
Expand Down Expand Up @@ -2869,9 +2870,6 @@ void dt_iop_refresh_center(dt_iop_module_t *module)
dt_develop_t *dev = module->dev;
if (dev && dev->gui_attached)
{
// invalidate the pixelpipe cache except for the output of the prior module
const uint64_t hash = dt_dev_pixelpipe_cache_basichash_prior(dev->pipe->image.id, dev->pipe, module);
dt_dev_pixelpipe_cache_flush_all_but(&dev->pipe->cache, hash);
dev->pipe->changed |= DT_DEV_PIPE_SYNCH; //ensure that commit_params gets called to pick up any GUI changes
dt_dev_invalidate(dev);
dt_control_queue_redraw_center();
Expand All @@ -2884,9 +2882,6 @@ void dt_iop_refresh_preview(dt_iop_module_t *module)
dt_develop_t *dev = module->dev;
if (dev && dev->gui_attached)
{
// invalidate the pixelpipe cache except for the output of the prior module
const uint64_t hash = dt_dev_pixelpipe_cache_basichash_prior(dev->pipe->image.id, dev->preview_pipe, module);
dt_dev_pixelpipe_cache_flush_all_but(&dev->preview_pipe->cache, hash);
dev->pipe->changed |= DT_DEV_PIPE_SYNCH; //ensure that commit_params gets called to pick up any GUI changes
dt_dev_invalidate_all(dev);
dt_control_queue_redraw();
Expand Down
111 changes: 4 additions & 107 deletions src/develop/pixelpipe_cache.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ int dt_dev_pixelpipe_cache_init(dt_dev_pixelpipe_cache_t *cache, int entries, si
#ifdef _DEBUG
memset(cache->dsc, 0x2c, sizeof(dt_iop_buffer_dsc_t) * entries);
#endif
cache->basichash = (uint64_t *)calloc(entries, sizeof(uint64_t));
cache->hash = (uint64_t *)calloc(entries, sizeof(uint64_t));
cache->used = (int32_t *)calloc(entries, sizeof(int32_t));
for(int k = 0; k < entries; k++)
Expand All @@ -58,7 +57,6 @@ int dt_dev_pixelpipe_cache_init(dt_dev_pixelpipe_cache_t *cache, int entries, si
ASAN_POISON_MEMORY_REGION(cache->data[k], cache->size[k]);
}
else cache->data[k] = 0;
cache->basichash[k] = -1;
cache->hash[k] = -1;
cache->used[k] = 0;
}
Expand All @@ -85,88 +83,11 @@ void dt_dev_pixelpipe_cache_cleanup(dt_dev_pixelpipe_cache_t *cache)
for(int k = 0; k < cache->entries; k++) dt_free_align(cache->data[k]);
free(cache->data);
free(cache->dsc);
free(cache->basichash);
free(cache->hash);
free(cache->used);
free(cache->size);
}

uint64_t dt_dev_pixelpipe_cache_basichash(int imgid, struct dt_dev_pixelpipe_t *pipe, int module)
{
// bernstein hash (djb2)
// the hash is made of imgid and the actual fast-pipe mode if activated
uint64_t hash = 5381 + imgid + (pipe->type & DT_DEV_PIXELPIPE_FAST);
// go through all modules up to module and compute a weird hash using the operation and params.
GList *pieces = pipe->nodes;
for(int k = 0; k < module && pieces; k++)
{
dt_dev_pixelpipe_iop_t *piece = (dt_dev_pixelpipe_iop_t *)pieces->data;
dt_develop_t *dev = piece->module->dev;
if(!(dev->gui_module && dev->gui_module != piece->module
&& (dev->gui_module->operation_tags_filter() & piece->module->operation_tags())))
{
hash = ((hash << 5) + hash) ^ piece->hash;
if(piece->module->request_color_pick != DT_REQUEST_COLORPICK_OFF)
{
if(darktable.lib->proxy.colorpicker.primary_sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
{
const char *str = (const char *)darktable.lib->proxy.colorpicker.primary_sample->box;
for(size_t i = 0; i < sizeof(float) * 4; i++) hash = ((hash << 5) + hash) ^ str[i];
}
else if(darktable.lib->proxy.colorpicker.primary_sample->size == DT_LIB_COLORPICKER_SIZE_POINT)
{
const char *str = (const char *)darktable.lib->proxy.colorpicker.primary_sample->point;
for(size_t i = 0; i < sizeof(float) * 2; i++) hash = ((hash << 5) + hash) ^ str[i];
}
}
}
pieces = g_list_next(pieces);
}
return hash;
}

uint64_t dt_dev_pixelpipe_cache_basichash_prior(int imgid, struct dt_dev_pixelpipe_t *pipe,
const dt_iop_module_t *const module)
{
;
// find the last enabled module prior to the specified one, then get its hash
GList *pieces = pipe->nodes;
GList *modules = pipe->iop;
int last = -1;
for(int k = 1; modules && pieces; k++)
{
dt_dev_pixelpipe_iop_t *piece = (dt_dev_pixelpipe_iop_t *)pieces->data;
if (module == (dt_iop_module_t *)modules->data)
break; // we've found the given module, so 'last' now contains the index of the prior active module
dt_develop_t *dev = piece->module->dev;
if(piece->enabled
&& !(dev->gui_module && dev->gui_module != piece->module
&& (dev->gui_module->operation_tags_filter() & piece->module->operation_tags())))
last = k;
pieces = g_list_next(pieces);
modules = g_list_next(modules);
}
return last>=0 ? dt_dev_pixelpipe_cache_basichash(imgid, pipe, last) : -1;
}

void dt_dev_pixelpipe_cache_fullhash(int imgid, const dt_iop_roi_t *roi, struct dt_dev_pixelpipe_t *pipe, int module,
uint64_t *basichash, uint64_t *fullhash)
{
uint64_t hash = *basichash = dt_dev_pixelpipe_cache_basichash(imgid, pipe, module);
// also add scale, x and y:
const char *str = (const char *)roi;
for(size_t i = 0; i < sizeof(dt_iop_roi_t); i++)
hash = ((hash << 5) + hash) ^ str[i];
*fullhash = hash;
}

uint64_t dt_dev_pixelpipe_cache_hash(int imgid, const dt_iop_roi_t *roi, dt_dev_pixelpipe_t *pipe, int module)
{
uint64_t basichash, hash;
dt_dev_pixelpipe_cache_fullhash(imgid, roi, pipe, module, &basichash, &hash);
return hash;
}

int dt_dev_pixelpipe_cache_available(dt_dev_pixelpipe_cache_t *cache, const uint64_t hash)
{
// search for hash in cache
Expand All @@ -175,20 +96,13 @@ int dt_dev_pixelpipe_cache_available(dt_dev_pixelpipe_cache_t *cache, const uint
return 0;
}

int dt_dev_pixelpipe_cache_get_important(dt_dev_pixelpipe_cache_t *cache, const uint64_t basichash,
const uint64_t hash, const size_t size,
void **data, dt_iop_buffer_dsc_t **dsc)
{
return dt_dev_pixelpipe_cache_get_weighted(cache, basichash, hash, size, data, dsc, -cache->entries);
}

int dt_dev_pixelpipe_cache_get(dt_dev_pixelpipe_cache_t *cache, const uint64_t basichash, const uint64_t hash,
int dt_dev_pixelpipe_cache_get(dt_dev_pixelpipe_cache_t *cache,const uint64_t hash,
const size_t size, void **data, dt_iop_buffer_dsc_t **dsc)
{
return dt_dev_pixelpipe_cache_get_weighted(cache, basichash, hash, size, data, dsc, 0);
return dt_dev_pixelpipe_cache_get_weighted(cache, hash, size, data, dsc, 0);
}

int dt_dev_pixelpipe_cache_get_weighted(dt_dev_pixelpipe_cache_t *cache, const uint64_t basichash, const uint64_t hash,
int dt_dev_pixelpipe_cache_get_weighted(dt_dev_pixelpipe_cache_t *cache, const uint64_t hash,
const size_t size, void **data, dt_iop_buffer_dsc_t **dsc, int weight)
{
cache->queries++;
Expand Down Expand Up @@ -237,7 +151,6 @@ int dt_dev_pixelpipe_cache_get_weighted(dt_dev_pixelpipe_cache_t *cache, const u
cache->dsc[max] = **dsc;
*dsc = &cache->dsc[max];

cache->basichash[max] = basichash;
cache->hash[max] = hash;
cache->used[max] = weight;
cache->misses++;
Expand All @@ -251,20 +164,6 @@ void dt_dev_pixelpipe_cache_flush(dt_dev_pixelpipe_cache_t *cache)
{
for(int k = 0; k < cache->entries; k++)
{
cache->basichash[k] = -1;
cache->hash[k] = -1;
cache->used[k] = 0;
ASAN_POISON_MEMORY_REGION(cache->data[k], cache->size[k]);
}
}

void dt_dev_pixelpipe_cache_flush_all_but(dt_dev_pixelpipe_cache_t *cache, uint64_t basichash)
{
for(int k = 0; k < cache->entries; k++)
{
if (cache->basichash[k] == basichash)
continue;
cache->basichash[k] = -1;
cache->hash[k] = -1;
cache->used[k] = 0;
ASAN_POISON_MEMORY_REGION(cache->data[k], cache->size[k]);
Expand All @@ -288,7 +187,6 @@ void dt_dev_pixelpipe_cache_invalidate(dt_dev_pixelpipe_cache_t *cache, void *da
{
if(cache->data[k] == data)
{
cache->basichash[k] = -1;
cache->hash[k] = -1;
ASAN_POISON_MEMORY_REGION(cache->data[k], cache->size[k]);
}
Expand All @@ -300,7 +198,7 @@ void dt_dev_pixelpipe_cache_print(dt_dev_pixelpipe_cache_t *cache)
for(int k = 0; k < cache->entries; k++)
{
printf("pixelpipe cacheline %d ", k);
printf("used %d by %" PRIu64 " (%" PRIu64 ")", cache->used[k], cache->hash[k], cache->basichash[k]);
printf("used %d by %" PRIu64 " (%" PRIu64 ")", cache->used[k], cache->hash[k], cache->hash[k]);
printf("\n");
}
printf("cache hit rate so far: %.3f\n", (cache->queries - cache->misses) / (float)cache->queries);
Expand All @@ -311,4 +209,3 @@ void dt_dev_pixelpipe_cache_print(dt_dev_pixelpipe_cache_t *cache)
// vim: shiftwidth=2 expandtab tabstop=2 cindent
// kate: tab-indents: off; indent-width 2; replace-tabs on; indent-mode cstyle; remove-trailing-spaces modified;
// clang-format on

24 changes: 2 additions & 22 deletions src/develop/pixelpipe_cache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ typedef struct dt_dev_pixelpipe_cache_t
void **data;
size_t *size;
struct dt_iop_buffer_dsc_t *dsc;
uint64_t *basichash;
uint64_t *hash;
int32_t *used;
#ifdef HAVE_OPENCL
Expand All @@ -53,27 +52,12 @@ typedef struct dt_dev_pixelpipe_cache_t
int dt_dev_pixelpipe_cache_init(dt_dev_pixelpipe_cache_t *cache, int entries, size_t size);
void dt_dev_pixelpipe_cache_cleanup(dt_dev_pixelpipe_cache_t *cache);

/** creates a hopefully unique hash from the complete module stack up to the module-th. */
uint64_t dt_dev_pixelpipe_cache_basichash(int imgid, struct dt_dev_pixelpipe_t *pipe, int module);
/** creates a hopefully unique hash from the complete module stack up to the module-th, including current viewport. */
uint64_t dt_dev_pixelpipe_cache_hash(int imgid, const struct dt_iop_roi_t *roi,
struct dt_dev_pixelpipe_t *pipe, int module);
/** return both of the above hashes */
void dt_dev_pixelpipe_cache_fullhash(int imgid, const dt_iop_roi_t *roi, struct dt_dev_pixelpipe_t *pipe, int module,
uint64_t *basichash, uint64_t *fullhash);
/** get the basichash for the last enabled module prior to the specified one */
uint64_t dt_dev_pixelpipe_cache_basichash_prior(int imgid, struct dt_dev_pixelpipe_t *pipe,
const struct dt_iop_module_t *const module);

/** returns the float data buffer for the given hash from the cache. if the hash does not match any
* cache line, the least recently used cache line will be cleared and an empty buffer is returned
* together with a non-zero return value. */
int dt_dev_pixelpipe_cache_get(dt_dev_pixelpipe_cache_t *cache, const uint64_t basichash, const uint64_t hash,
int dt_dev_pixelpipe_cache_get(dt_dev_pixelpipe_cache_t *cache, const uint64_t hash,
const size_t size, void **data, struct dt_iop_buffer_dsc_t **dsc);
int dt_dev_pixelpipe_cache_get_important(dt_dev_pixelpipe_cache_t *cache, const uint64_t basichash,
const uint64_t hash, const size_t size,
void **data, struct dt_iop_buffer_dsc_t **dsc);
int dt_dev_pixelpipe_cache_get_weighted(dt_dev_pixelpipe_cache_t *cache, const uint64_t basichash,
int dt_dev_pixelpipe_cache_get_weighted(dt_dev_pixelpipe_cache_t *cache,
const uint64_t hash, const size_t size,
void **data, struct dt_iop_buffer_dsc_t **dsc, int weight);

Expand All @@ -83,9 +67,6 @@ int dt_dev_pixelpipe_cache_available(dt_dev_pixelpipe_cache_t *cache, const uint
/** invalidates all cachelines. */
void dt_dev_pixelpipe_cache_flush(dt_dev_pixelpipe_cache_t *cache);

/** invalidates all cachelines except those containing items for the given module/parameter combination */
void dt_dev_pixelpipe_cache_flush_all_but(dt_dev_pixelpipe_cache_t *cache, uint64_t basichash);

/** makes this buffer very important after it has been pulled from the cache. */
void dt_dev_pixelpipe_cache_reweight(dt_dev_pixelpipe_cache_t *cache, void *data);

Expand All @@ -100,4 +81,3 @@ void dt_dev_pixelpipe_cache_print(dt_dev_pixelpipe_cache_t *cache);
// vim: shiftwidth=2 expandtab tabstop=2 cindent
// kate: tab-indents: off; indent-width 2; replace-tabs on; indent-mode cstyle; remove-trailing-spaces modified;
// clang-format on

Loading

0 comments on commit a95375f

Please sign in to comment.