[Cogl] Shader garbage collection
Robert Bragg
robert at sixbynine.org
Mon Jan 13 10:44:49 PST 2014
Sorry for the delay in reviewing this patch.
The patch looks really good to me!
Reviewed-by: Robert Bragg <robert at linux.intel.com>
Thanks,
Robert
On Wed, Dec 18, 2013 at 3:18 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Yeah, I think you're right and it would be a good idea to land this
> patch in order to support the shader editor use case. I don't think
> the garbage collection should make other use cases any worse because
> it only kicks in when Cogl is about to compile a new shader anyway and
> any processing it is going to do will probably be dwarfed by the
> processing needed to do the compilation.
>
> Here is a rebased version of the patch with the addition of a white
> box unix test to make sure that the garbage collection does actually
> kick in.
>
> ------- >8 --------------- (use git am --scissors to automatically chop here)
> Subject: pipeline-cache: Prune old unused pipelines when the cache gets too big
>
> Previously when a pipeline is added to the cache it would never be
> removed. If the application is generating a lot of unique pipelines
> this can end up effectively leaking a large number of resources
> including the GL program objects. Arguably this isn't really a problem
> because if the application is generating that many unique pipelines
> then it is doing something wrong anyway. It also implies that it will
> be recompiling shaders very often so the cache leaking will likely be
> the least of the problems.
>
> This patch makes it keep track of which pipelines in the cache are in
> use. The cache now returns a struct representing the entry instead of
> directly returning the pipeline. This entry contains a usage counter
> which the pipeline backends can use to mark when there is a pipeline
> alive that is using the cache entry. When the hash table decides that
> it's a good time to prune some entries, it will make a list of all of
> the pipelines that are not in use and then remove the least recently
> used half of the pipelines. That way it is less likely to remove
> pipelines that the application is actually regenerating often even if
> they aren't in use all of the time.
>
> When the cache is pruned the hash table makes a note of how small the
> cache could be if it removed all of the unused pipelines. The hash
> table starts pruning when there are more entries than twice this
> minimum expected size. The idea is that if that case it hit then the
> hash table is more than half full of useless pipelines so the
> application is generating lots of redundant pipelines and it is a good
> time to remove them.
> ---
> cogl/cogl-pipeline-cache.c | 105 +++++++++++++++++++++++-
> cogl/cogl-pipeline-cache.h | 15 +++-
> cogl/cogl-pipeline-hash-table.c | 98 +++++++++++++++++++---
> cogl/cogl-pipeline-hash-table.h | 9 +-
> cogl/driver/gl/cogl-pipeline-fragend-glsl.c | 54 +++++++-----
> cogl/driver/gl/cogl-pipeline-progend-glsl.c | 44 ++++++----
> cogl/driver/gl/cogl-pipeline-vertend-glsl.c | 52 +++++++-----
> cogl/driver/gl/gl/cogl-pipeline-fragend-arbfp.c | 44 ++++++----
> 8 files changed, 334 insertions(+), 87 deletions(-)
>
> diff --git a/cogl/cogl-pipeline-cache.c b/cogl/cogl-pipeline-cache.c
> index 3a96490..cc4e5c8 100644
> --- a/cogl/cogl-pipeline-cache.c
> +++ b/cogl/cogl-pipeline-cache.c
> @@ -29,6 +29,8 @@
> #include "config.h"
> #endif
>
> +#include <test-fixtures/test-unit.h>
> +
> #include "cogl-context-private.h"
> #include "cogl-pipeline-private.h"
> #include "cogl-pipeline-cache.h"
> @@ -86,7 +88,7 @@ _cogl_pipeline_cache_free (CoglPipelineCache *cache)
> g_free (cache);
> }
>
> -CoglPipeline *
> +CoglPipelineCacheEntry *
> _cogl_pipeline_cache_get_fragment_template (CoglPipelineCache *cache,
> CoglPipeline *key_pipeline)
> {
> @@ -94,7 +96,7 @@ _cogl_pipeline_cache_get_fragment_template (CoglPipelineCache *cache,
> key_pipeline);
> }
>
> -CoglPipeline *
> +CoglPipelineCacheEntry *
> _cogl_pipeline_cache_get_vertex_template (CoglPipelineCache *cache,
> CoglPipeline *key_pipeline)
> {
> @@ -102,10 +104,107 @@ _cogl_pipeline_cache_get_vertex_template (CoglPipelineCache *cache,
> key_pipeline);
> }
>
> -CoglPipeline *
> +CoglPipelineCacheEntry *
> _cogl_pipeline_cache_get_combined_template (CoglPipelineCache *cache,
> CoglPipeline *key_pipeline)
> {
> return _cogl_pipeline_hash_table_get (&cache->combined_hash,
> key_pipeline);
> }
> +
> +#ifdef ENABLE_UNIT_TESTS
> +
> +static void
> +create_pipelines (CoglPipeline **pipelines,
> + int n_pipelines)
> +{
> + int i;
> +
> + for (i = 0; i < n_pipelines; i++)
> + {
> + char *source = g_strdup_printf (" cogl_color_out = "
> + "vec4 (%f, 0.0, 0.0, 1.0);\n",
> + i / 255.0f);
> + CoglSnippet *snippet =
> + cogl_snippet_new (COGL_SNIPPET_HOOK_FRAGMENT,
> + NULL, /* declarations */
> + source);
> +
> + g_free (source);
> +
> + pipelines[i] = cogl_pipeline_new (test_ctx);
> + cogl_pipeline_add_snippet (pipelines[i], snippet);
> + cogl_object_unref (snippet);
> + }
> +
> + /* Test that drawing with them works. This should create the entries
> + * in the cache */
> + for (i = 0; i < n_pipelines; i++)
> + {
> + cogl_framebuffer_draw_rectangle (test_fb,
> + pipelines[i],
> + i, 0,
> + i + 1, 1);
> + test_utils_check_pixel_rgb (test_fb, i, 0, i, 0, 0);
> + }
> +
> +}
> +
> +UNIT_TEST (check_pipeline_pruning,
> + TEST_REQUIREMENT_GLSL, /* requirements */
> + 0 /* no failure cases */)
> +{
> + CoglPipeline *pipelines[18];
> + int fb_width, fb_height;
> + CoglPipelineHashTable *fragment_hash =
> + &test_ctx->pipeline_cache->fragment_hash;
> + CoglPipelineHashTable *combined_hash =
> + &test_ctx->pipeline_cache->combined_hash;
> + int i;
> +
> + fb_width = cogl_framebuffer_get_width (test_fb);
> + fb_height = cogl_framebuffer_get_height (test_fb);
> +
> + cogl_framebuffer_orthographic (test_fb,
> + 0, 0,
> + fb_width,
> + fb_height,
> + -1,
> + 100);
> +
> + /* Create 18 unique pipelines. This should end up being more than
> + * the initial expected minimum size so it will trigger the garbage
> + * collection. However all of the pipelines will be in use so they
> + * won't be collected */
> + create_pipelines (pipelines, 18);
> +
> + /* These pipelines should all have unique entries in the cache. We
> + * should have run the garbage collection once and at that point the
> + * expected minimum size would have been 17 */
> + g_assert_cmpint (g_hash_table_size (fragment_hash->table), ==, 18);
> + g_assert_cmpint (g_hash_table_size (combined_hash->table), ==, 18);
> + g_assert_cmpint (fragment_hash->expected_min_size, ==, 17);
> + g_assert_cmpint (combined_hash->expected_min_size, ==, 17);
> +
> + /* Destroy the original pipelines and create some new ones. This
> + * should run the garbage collector again but this time the
> + * pipelines won't be in use so it should free some of them */
> + for (i = 0; i < 18; i++)
> + cogl_object_unref (pipelines[i]);
> +
> + create_pipelines (pipelines, 18);
> +
> + /* The garbage collection should have freed half of the original 18
> + * pipelines which means there should now be 18*1.5 = 27 */
> + g_assert_cmpint (g_hash_table_size (fragment_hash->table), ==, 27);
> + g_assert_cmpint (g_hash_table_size (combined_hash->table), ==, 27);
> + /* The 35th pipeline would have caused the garbage collection. At
> + * that point there would be 35-18=17 used unique pipelines. */
> + g_assert_cmpint (fragment_hash->expected_min_size, ==, 17);
> + g_assert_cmpint (combined_hash->expected_min_size, ==, 17);
> +
> + for (i = 0; i < 18; i++)
> + cogl_object_unref (pipelines[i]);
> +}
> +
> +#endif /* ENABLE_UNIT_TESTS */
> diff --git a/cogl/cogl-pipeline-cache.h b/cogl/cogl-pipeline-cache.h
> index 6bc15fd..7bb31ea 100644
> --- a/cogl/cogl-pipeline-cache.h
> +++ b/cogl/cogl-pipeline-cache.h
> @@ -28,6 +28,15 @@
>
> typedef struct _CoglPipelineCache CoglPipelineCache;
>
> +typedef struct
> +{
> + CoglPipeline *pipeline;
> +
> + /* Number of usages of this template. If this drops to zero then it
> + * will be a candidate for removal from the cache */
> + int usage_count;
> +} CoglPipelineCacheEntry;
> +
> CoglPipelineCache *
> _cogl_pipeline_cache_new (void);
>
> @@ -43,7 +52,7 @@ _cogl_pipeline_cache_free (CoglPipelineCache *cache);
> * with a similar pipeline. In that case the copy itself will be
> * returned
> */
> -CoglPipeline *
> +CoglPipelineCacheEntry *
> _cogl_pipeline_cache_get_fragment_template (CoglPipelineCache *cache,
> CoglPipeline *key_pipeline);
>
> @@ -56,7 +65,7 @@ _cogl_pipeline_cache_get_fragment_template (CoglPipelineCache *cache,
> * with a similar pipeline. In that case the copy itself will be
> * returned
> */
> -CoglPipeline *
> +CoglPipelineCacheEntry *
> _cogl_pipeline_cache_get_vertex_template (CoglPipelineCache *cache,
> CoglPipeline *key_pipeline);
>
> @@ -70,7 +79,7 @@ _cogl_pipeline_cache_get_vertex_template (CoglPipelineCache *cache,
> * with a similar pipeline. In that case the copy itself will be
> * returned
> */
> -CoglPipeline *
> +CoglPipelineCacheEntry *
> _cogl_pipeline_cache_get_combined_template (CoglPipelineCache *cache,
> CoglPipeline *key_pipeline);
>
> diff --git a/cogl/cogl-pipeline-hash-table.c b/cogl/cogl-pipeline-hash-table.c
> index 8921efc..84c5672 100644
> --- a/cogl/cogl-pipeline-hash-table.c
> +++ b/cogl/cogl-pipeline-hash-table.c
> @@ -32,11 +32,11 @@
> #include "cogl-context-private.h"
> #include "cogl-pipeline-private.h"
> #include "cogl-pipeline-hash-table.h"
> +#include "cogl-pipeline-cache.h"
>
> typedef struct
> {
> - /* The template pipeline */
> - CoglPipeline *pipeline;
> + CoglPipelineCacheEntry parent;
>
> /* Calculating the hash is a little bit expensive for pipelines so
> * we don't want to do it repeatedly for entries that are already in
> @@ -49,6 +49,10 @@ typedef struct
> * store the pointer in every hash table entry. We will use this
> * entry as both the key and the value */
> CoglPipelineHashTable *hash;
> +
> + /* The number of unique pipelines that had been created when this
> + * pipeline was last accessed */
> + int age;
> } CoglPipelineHashTableEntry;
>
> static void
> @@ -56,7 +60,7 @@ value_destroy_cb (void *value)
> {
> CoglPipelineHashTableEntry *entry = value;
>
> - cogl_object_unref (entry->pipeline);
> + cogl_object_unref (entry->parent.pipeline);
>
> g_slice_free (CoglPipelineHashTableEntry, entry);
> }
> @@ -77,8 +81,8 @@ entry_equal (const void *a,
> const CoglPipelineHashTableEntry *entry_b = b;
> const CoglPipelineHashTable *hash = entry_a->hash;
>
> - return _cogl_pipeline_equal (entry_a->pipeline,
> - entry_b->pipeline,
> + return _cogl_pipeline_equal (entry_a->parent.pipeline,
> + entry_b->parent.pipeline,
> hash->main_state,
> hash->layer_state,
> 0);
> @@ -94,6 +98,8 @@ _cogl_pipeline_hash_table_init (CoglPipelineHashTable *hash,
> hash->debug_string = debug_string;
> hash->main_state = main_state;
> hash->layer_state = layer_state;
> + /* We'll only start pruning once we get to 16 unique pipelines */
> + hash->expected_min_size = 8;
> hash->table = g_hash_table_new_full (entry_hash,
> entry_equal,
> NULL, /* key destroy */
> @@ -106,7 +112,64 @@ _cogl_pipeline_hash_table_destroy (CoglPipelineHashTable *hash)
> g_hash_table_destroy (hash->table);
> }
>
> -CoglPipeline *
> +static void
> +collect_prunable_entries_cb (void *key,
> + void *value,
> + void *user_data)
> +{
> + GQueue *entries = user_data;
> + CoglPipelineCacheEntry *entry = value;
> +
> + if (entry->usage_count == 0)
> + g_queue_push_tail (entries, entry);
> +}
> +
> +static int
> +compare_pipeline_age_cb (const void *a,
> + const void *b)
> +{
> + const CoglPipelineHashTableEntry *ae = a;
> + const CoglPipelineHashTableEntry *be = b;
> +
> + return be->age - ae->age;
> +}
> +
> +static void
> +prune_old_pipelines (CoglPipelineHashTable *hash)
> +{
> + GQueue entries;
> + GList *l;
> + int i;
> +
> + /* Collect all of the prunable entries into a GQueue */
> + g_queue_init (&entries);
> + g_hash_table_foreach (hash->table,
> + collect_prunable_entries_cb,
> + &entries);
> +
> + /* Sort the entries by increasing order of age */
> + entries.head = g_list_sort (entries.head, compare_pipeline_age_cb);
> +
> + /* The +1 is to include the pipeline that we're about to add */
> + hash->expected_min_size = (g_hash_table_size (hash->table) -
> + entries.length +
> + 1);
> +
> + /* Remove oldest half of the prunable pipelines. We still want to
> + * keep some of the prunable entries that are recently used because
> + * it's not unlikely that the application will recreate the same
> + * pipeline */
> + for (l = entries.head, i = 0; i < entries.length / 2; l = l->next, i++)
> + {
> + CoglPipelineCacheEntry *entry = l->data;
> +
> + g_hash_table_remove (hash->table, entry);
> + }
> +
> + g_list_free (entries.head);
> +}
> +
> +CoglPipelineCacheEntry *
> _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash,
> CoglPipeline *key_pipeline)
> {
> @@ -114,7 +177,7 @@ _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash,
> CoglPipelineHashTableEntry *entry;
> unsigned int copy_state;
>
> - dummy_entry.pipeline = key_pipeline;
> + dummy_entry.parent.pipeline = key_pipeline;
> dummy_entry.hash = hash;
> dummy_entry.hash_value = _cogl_pipeline_hash (key_pipeline,
> hash->main_state,
> @@ -123,16 +186,27 @@ _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash,
> entry = g_hash_table_lookup (hash->table, &dummy_entry);
>
> if (entry)
> - return entry->pipeline;
> + {
> + entry->age = hash->n_unique_pipelines;
> + return &entry->parent;
> + }
>
> if (hash->n_unique_pipelines == 50)
> g_warning ("Over 50 separate %s have been generated which is very "
> "unusual, so something is probably wrong!\n",
> hash->debug_string);
>
> + /* If we are going to have more than twice the expected minimum
> + * number of pipelines in the hash then we'll try pruning and update
> + * the minimum */
> + if (g_hash_table_size (hash->table) >= hash->expected_min_size * 2)
> + prune_old_pipelines (hash);
> +
> entry = g_slice_new (CoglPipelineHashTableEntry);
> + entry->parent.usage_count = 0;
> entry->hash = hash;
> entry->hash_value = dummy_entry.hash_value;
> + entry->age = hash->n_unique_pipelines;
>
> copy_state = hash->main_state;
> if (hash->layer_state)
> @@ -141,13 +215,13 @@ _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash,
> /* Create a new pipeline that is a child of the root pipeline
> * instead of a normal copy so that the template pipeline won't hold
> * a reference to the original pipeline */
> - entry->pipeline = _cogl_pipeline_deep_copy (key_pipeline,
> - copy_state,
> - hash->layer_state);
> + entry->parent.pipeline = _cogl_pipeline_deep_copy (key_pipeline,
> + copy_state,
> + hash->layer_state);
>
> g_hash_table_insert (hash->table, entry, entry);
>
> hash->n_unique_pipelines++;
>
> - return entry->pipeline;
> + return &entry->parent;
> }
> diff --git a/cogl/cogl-pipeline-hash-table.h b/cogl/cogl-pipeline-hash-table.h
> index 1b0a0d9..0f3d37c 100644
> --- a/cogl/cogl-pipeline-hash-table.h
> +++ b/cogl/cogl-pipeline-hash-table.h
> @@ -24,7 +24,7 @@
> #ifndef __COGL_PIPELINE_HASH_H__
> #define __COGL_PIPELINE_HASH_H__
>
> -#include "cogl-pipeline.h"
> +#include "cogl-pipeline-cache.h"
>
> typedef struct
> {
> @@ -34,6 +34,11 @@ typedef struct
> * generated */
> int n_unique_pipelines;
>
> + /* This is the expected minimum size we could prune the hash table
> + * to if we were to remove all pipelines that are not in use. This
> + * is only updated after we prune the table */
> + int expected_min_size;
> +
> /* String that will be used to describe the usage of this hash table
> * in the debug warning when too many pipelines are generated. This
> * must be a static string because it won't be copied or freed */
> @@ -62,7 +67,7 @@ _cogl_pipeline_hash_table_destroy (CoglPipelineHashTable *hash);
> * it will be used next time the function is called with a similar
> * pipeline. In that case the copy itself will be returned
> */
> -CoglPipeline *
> +CoglPipelineCacheEntry *
> _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash,
> CoglPipeline *key_pipeline);
>
> diff --git a/cogl/driver/gl/cogl-pipeline-fragend-glsl.c b/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> index 687a826..9d088eb 100644
> --- a/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> +++ b/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> @@ -93,6 +93,8 @@ typedef struct
> layer we'll remove it from the list so we don't generate it
> again */
> CoglList layers;
> +
> + CoglPipelineCacheEntry *cache_entry;
> } CoglPipelineShaderState;
>
> static CoglUserDataKey shader_state_key;
> @@ -102,13 +104,15 @@ ensure_layer_generated (CoglPipeline *pipeline,
> int layer_num);
>
> static CoglPipelineShaderState *
> -shader_state_new (int n_layers)
> +shader_state_new (int n_layers,
> + CoglPipelineCacheEntry *cache_entry)
> {
> CoglPipelineShaderState *shader_state;
>
> shader_state = g_slice_new0 (CoglPipelineShaderState);
> shader_state->ref_count = 1;
> shader_state->unit_state = g_new0 (UnitState, n_layers);
> + shader_state->cache_entry = cache_entry;
>
> return shader_state;
> }
> @@ -120,12 +124,17 @@ get_shader_state (CoglPipeline *pipeline)
> }
>
> static void
> -destroy_shader_state (void *user_data)
> +destroy_shader_state (void *user_data,
> + void *instance)
> {
> CoglPipelineShaderState *shader_state = user_data;
>
> _COGL_GET_CONTEXT (ctx, NO_RETVAL);
>
> + if (shader_state->cache_entry &&
> + shader_state->cache_entry->pipeline != instance)
> + shader_state->cache_entry->usage_count--;
> +
> if (--shader_state->ref_count == 0)
> {
> if (shader_state->gl_shader)
> @@ -140,10 +149,21 @@ destroy_shader_state (void *user_data)
> static void
> set_shader_state (CoglPipeline *pipeline, CoglPipelineShaderState *shader_state)
> {
> - cogl_object_set_user_data (COGL_OBJECT (pipeline),
> - &shader_state_key,
> - shader_state,
> - destroy_shader_state);
> + if (shader_state)
> + {
> + shader_state->ref_count++;
> +
> + /* If we're not setting the state on the template pipeline then
> + * mark it as a usage of the pipeline cache entry */
> + if (shader_state->cache_entry &&
> + shader_state->cache_entry->pipeline != pipeline)
> + shader_state->cache_entry->usage_count++;
> + }
> +
> + _cogl_object_set_user_data (COGL_OBJECT (pipeline),
> + &shader_state_key,
> + shader_state,
> + destroy_shader_state);
> }
>
> static void
> @@ -260,7 +280,7 @@ _cogl_pipeline_fragend_glsl_start (CoglPipeline *pipeline,
> {
> CoglPipelineShaderState *shader_state;
> CoglPipeline *authority;
> - CoglPipeline *template_pipeline = NULL;
> + CoglPipelineCacheEntry *cache_entry = NULL;
> int i;
>
> _COGL_GET_CONTEXT (ctx, NO_RETVAL);
> @@ -296,35 +316,31 @@ _cogl_pipeline_fragend_glsl_start (CoglPipeline *pipeline,
> if (G_LIKELY (!(COGL_DEBUG_ENABLED
> (COGL_DEBUG_DISABLE_PROGRAM_CACHES))))
> {
> - template_pipeline =
> + cache_entry =
> _cogl_pipeline_cache_get_fragment_template (ctx->pipeline_cache,
> authority);
>
> - shader_state = get_shader_state (template_pipeline);
> + shader_state = get_shader_state (cache_entry->pipeline);
> }
>
> if (shader_state)
> shader_state->ref_count++;
> else
> - shader_state = shader_state_new (n_layers);
> + shader_state = shader_state_new (n_layers, cache_entry);
>
> set_shader_state (authority, shader_state);
>
> - if (template_pipeline)
> - {
> - shader_state->ref_count++;
> - set_shader_state (template_pipeline, shader_state);
> - }
> + shader_state->ref_count--;
> +
> + if (cache_entry)
> + set_shader_state (cache_entry->pipeline, shader_state);
> }
>
> /* If the pipeline isn't actually its own glsl-authority
> * then take a reference to the program state associated
> * with the glsl-authority... */
> if (authority != pipeline)
> - {
> - shader_state->ref_count++;
> - set_shader_state (pipeline, shader_state);
> - }
> + set_shader_state (pipeline, shader_state);
> }
>
> if (shader_state->gl_shader)
> diff --git a/cogl/driver/gl/cogl-pipeline-progend-glsl.c b/cogl/driver/gl/cogl-pipeline-progend-glsl.c
> index c4ca385..9f050be 100644
> --- a/cogl/driver/gl/cogl-pipeline-progend-glsl.c
> +++ b/cogl/driver/gl/cogl-pipeline-progend-glsl.c
> @@ -131,6 +131,8 @@ typedef struct
> int flushed_flip_state;
>
> UnitState *unit_state;
> +
> + CoglPipelineCacheEntry *cache_entry;
> } CoglPipelineProgramState;
>
> static CoglUserDataKey program_state_key;
> @@ -220,7 +222,8 @@ clear_flushed_matrix_stacks (CoglPipelineProgramState *program_state)
> }
>
> static CoglPipelineProgramState *
> -program_state_new (int n_layers)
> +program_state_new (int n_layers,
> + CoglPipelineCacheEntry *cache_entry)
> {
> CoglPipelineProgramState *program_state;
>
> @@ -230,6 +233,7 @@ program_state_new (int n_layers)
> program_state->unit_state = g_new (UnitState, n_layers);
> program_state->uniform_locations = NULL;
> program_state->attribute_locations = NULL;
> + program_state->cache_entry = cache_entry;
> _cogl_matrix_entry_cache_init (&program_state->modelview_cache);
> _cogl_matrix_entry_cache_init (&program_state->projection_cache);
>
> @@ -251,6 +255,10 @@ destroy_program_state (void *user_data,
> if (program_state->last_used_for_pipeline == instance)
> program_state->last_used_for_pipeline = NULL;
>
> + if (program_state->cache_entry &&
> + program_state->cache_entry->pipeline != instance)
> + program_state->cache_entry->usage_count--;
> +
> if (--program_state->ref_count == 0)
> {
> clear_attribute_cache (program_state);
> @@ -274,6 +282,17 @@ static void
> set_program_state (CoglPipeline *pipeline,
> CoglPipelineProgramState *program_state)
> {
> + if (program_state)
> + {
> + program_state->ref_count++;
> +
> + /* If we're not setting the state on the template pipeline then
> + * mark it as a usage of the pipeline cache entry */
> + if (program_state->cache_entry &&
> + program_state->cache_entry->pipeline != pipeline)
> + program_state->cache_entry->usage_count++;
> + }
> +
> _cogl_object_set_user_data (COGL_OBJECT (pipeline),
> &program_state_key,
> program_state,
> @@ -602,7 +621,7 @@ _cogl_pipeline_progend_glsl_end (CoglPipeline *pipeline,
> GLuint gl_program;
> CoglBool program_changed = FALSE;
> UpdateUniformsState state;
> - CoglPipeline *template_pipeline = NULL;
> + CoglPipelineCacheEntry *cache_entry = NULL;
>
> _COGL_GET_CONTEXT (ctx, NO_RETVAL);
>
> @@ -632,33 +651,30 @@ _cogl_pipeline_progend_glsl_end (CoglPipeline *pipeline,
> if (G_LIKELY (!(COGL_DEBUG_ENABLED
> (COGL_DEBUG_DISABLE_PROGRAM_CACHES))))
> {
> - template_pipeline =
> + cache_entry =
> _cogl_pipeline_cache_get_combined_template (ctx->pipeline_cache,
> authority);
>
> - program_state = get_program_state (template_pipeline);
> + program_state = get_program_state (cache_entry->pipeline);
> }
>
> if (program_state)
> program_state->ref_count++;
> else
> program_state
> - = program_state_new (cogl_pipeline_get_n_layers (authority));
> + = program_state_new (cogl_pipeline_get_n_layers (authority),
> + cache_entry);
>
> set_program_state (authority, program_state);
>
> - if (template_pipeline)
> - {
> - program_state->ref_count++;
> - set_program_state (template_pipeline, program_state);
> - }
> + program_state->ref_count--;
> +
> + if (cache_entry)
> + set_program_state (cache_entry->pipeline, program_state);
> }
>
> if (authority != pipeline)
> - {
> - program_state->ref_count++;
> - set_program_state (pipeline, program_state);
> - }
> + set_program_state (pipeline, program_state);
> }
>
> if (program_state->program == 0)
> diff --git a/cogl/driver/gl/cogl-pipeline-vertend-glsl.c b/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
> index 4f5587d..e2a7d00 100644
> --- a/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
> +++ b/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
> @@ -55,17 +55,19 @@ typedef struct
> GLuint gl_shader;
> GString *header, *source;
>
> + CoglPipelineCacheEntry *cache_entry;
> } CoglPipelineShaderState;
>
> static CoglUserDataKey shader_state_key;
>
> static CoglPipelineShaderState *
> -shader_state_new (void)
> +shader_state_new (CoglPipelineCacheEntry *cache_entry)
> {
> CoglPipelineShaderState *shader_state;
>
> shader_state = g_slice_new0 (CoglPipelineShaderState);
> shader_state->ref_count = 1;
> + shader_state->cache_entry = cache_entry;
>
> return shader_state;
> }
> @@ -77,12 +79,17 @@ get_shader_state (CoglPipeline *pipeline)
> }
>
> static void
> -destroy_shader_state (void *user_data)
> +destroy_shader_state (void *user_data,
> + void *instance)
> {
> CoglPipelineShaderState *shader_state = user_data;
>
> _COGL_GET_CONTEXT (ctx, NO_RETVAL);
>
> + if (shader_state->cache_entry &&
> + shader_state->cache_entry->pipeline != instance)
> + shader_state->cache_entry->usage_count--;
> +
> if (--shader_state->ref_count == 0)
> {
> if (shader_state->gl_shader)
> @@ -96,10 +103,21 @@ static void
> set_shader_state (CoglPipeline *pipeline,
> CoglPipelineShaderState *shader_state)
> {
> - cogl_object_set_user_data (COGL_OBJECT (pipeline),
> - &shader_state_key,
> - shader_state,
> - destroy_shader_state);
> + if (shader_state)
> + {
> + shader_state->ref_count++;
> +
> + /* If we're not setting the state on the template pipeline then
> + * mark it as a usage of the pipeline cache entry */
> + if (shader_state->cache_entry &&
> + shader_state->cache_entry->pipeline != pipeline)
> + shader_state->cache_entry->usage_count++;
> + }
> +
> + _cogl_object_set_user_data (COGL_OBJECT (pipeline),
> + &shader_state_key,
> + shader_state,
> + destroy_shader_state);
> }
>
> static void
> @@ -200,7 +218,7 @@ _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline,
> unsigned long pipelines_difference)
> {
> CoglPipelineShaderState *shader_state;
> - CoglPipeline *template_pipeline = NULL;
> + CoglPipelineCacheEntry *cache_entry = NULL;
>
> _COGL_GET_CONTEXT (ctx, NO_RETVAL);
>
> @@ -229,32 +247,28 @@ _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline,
> if (G_LIKELY (!(COGL_DEBUG_ENABLED
> (COGL_DEBUG_DISABLE_PROGRAM_CACHES))))
> {
> - template_pipeline =
> + cache_entry =
> _cogl_pipeline_cache_get_vertex_template (ctx->pipeline_cache,
> authority);
>
> - shader_state = get_shader_state (template_pipeline);
> + shader_state = get_shader_state (cache_entry->pipeline);
> }
>
> if (shader_state)
> shader_state->ref_count++;
> else
> - shader_state = shader_state_new ();
> + shader_state = shader_state_new (cache_entry);
>
> set_shader_state (authority, shader_state);
>
> - if (template_pipeline)
> - {
> - shader_state->ref_count++;
> - set_shader_state (template_pipeline, shader_state);
> - }
> + shader_state->ref_count--;
> +
> + if (cache_entry)
> + set_shader_state (cache_entry->pipeline, shader_state);
> }
>
> if (authority != pipeline)
> - {
> - shader_state->ref_count++;
> - set_shader_state (pipeline, shader_state);
> - }
> + set_shader_state (pipeline, shader_state);
> }
>
> if (shader_state->gl_shader)
> diff --git a/cogl/driver/gl/gl/cogl-pipeline-fragend-arbfp.c b/cogl/driver/gl/gl/cogl-pipeline-fragend-arbfp.c
> index 1325fd9..20a11a0 100644
> --- a/cogl/driver/gl/gl/cogl-pipeline-fragend-arbfp.c
> +++ b/cogl/driver/gl/gl/cogl-pipeline-fragend-arbfp.c
> @@ -80,18 +80,22 @@ typedef struct
> /* We need to track the last pipeline that an ARBfp program was used
> * with so know if we need to update any program.local parameters. */
> CoglPipeline *last_used_for_pipeline;
> +
> + CoglPipelineCacheEntry *cache_entry;
> } CoglPipelineShaderState;
>
> static CoglUserDataKey shader_state_key;
>
> static CoglPipelineShaderState *
> -shader_state_new (int n_layers)
> +shader_state_new (int n_layers,
> + CoglPipelineCacheEntry *cache_entry)
> {
> CoglPipelineShaderState *shader_state;
>
> shader_state = g_slice_new0 (CoglPipelineShaderState);
> shader_state->ref_count = 1;
> shader_state->unit_state = g_new0 (UnitState, n_layers);
> + shader_state->cache_entry = cache_entry;
>
> return shader_state;
> }
> @@ -117,6 +121,10 @@ destroy_shader_state (void *user_data,
> if (shader_state->last_used_for_pipeline == instance)
> shader_state->last_used_for_pipeline = NULL;
>
> + if (shader_state->cache_entry &&
> + shader_state->cache_entry->pipeline != instance)
> + shader_state->cache_entry->usage_count--;
> +
> if (--shader_state->ref_count == 0)
> {
> if (shader_state->gl_program)
> @@ -134,6 +142,17 @@ destroy_shader_state (void *user_data,
> static void
> set_shader_state (CoglPipeline *pipeline, CoglPipelineShaderState *shader_state)
> {
> + if (shader_state)
> + {
> + shader_state->ref_count++;
> +
> + /* If we're not setting the state on the template pipeline then
> + * mark it as a usage of the pipeline cache entry */
> + if (shader_state->cache_entry &&
> + shader_state->cache_entry->pipeline != pipeline)
> + shader_state->cache_entry->usage_count++;
> + }
> +
> _cogl_object_set_user_data (COGL_OBJECT (pipeline),
> &shader_state_key,
> shader_state,
> @@ -156,7 +175,7 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
> {
> CoglPipelineShaderState *shader_state;
> CoglPipeline *authority;
> - CoglPipeline *template_pipeline = NULL;
> + CoglPipelineCacheEntry *cache_entry = NULL;
>
> _COGL_GET_CONTEXT (ctx, NO_RETVAL);
>
> @@ -187,7 +206,6 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
> /* If we are going to share our program state with an arbfp-authority
> * then add a reference to the program state associated with that
> * arbfp-authority... */
> - shader_state->ref_count++;
> set_shader_state (pipeline, shader_state);
> return;
> }
> @@ -197,11 +215,11 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
> * program in the pipeline_cache. */
> if (G_LIKELY (!(COGL_DEBUG_ENABLED (COGL_DEBUG_DISABLE_PROGRAM_CACHES))))
> {
> - template_pipeline =
> + cache_entry =
> _cogl_pipeline_cache_get_fragment_template (ctx->pipeline_cache,
> authority);
>
> - shader_state = get_shader_state (template_pipeline);
> + shader_state = get_shader_state (cache_entry->pipeline);
>
> if (shader_state)
> shader_state->ref_count++;
> @@ -211,7 +229,7 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
> a new one */
> if (shader_state == NULL)
> {
> - shader_state = shader_state_new (n_layers);
> + shader_state = shader_state_new (n_layers, cache_entry);
>
> /* We reuse a single grow-only GString for code-gen */
> g_string_set_size (ctx->codegen_source_buffer, 0);
> @@ -228,22 +246,18 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
>
> set_shader_state (pipeline, shader_state);
>
> + shader_state->ref_count--;
> +
> /* Since we have already resolved the arbfp-authority at this point
> * we might as well also associate any program we find from the cache
> * with the authority too... */
> if (authority != pipeline)
> - {
> - shader_state->ref_count++;
> - set_shader_state (authority, shader_state);
> - }
> + set_shader_state (authority, shader_state);
>
> /* If we found a template then we'll attach it to that too so that
> next time a similar pipeline is used it can use the same state */
> - if (template_pipeline)
> - {
> - shader_state->ref_count++;
> - set_shader_state (template_pipeline, shader_state);
> - }
> + if (cache_entry)
> + set_shader_state (cache_entry->pipeline, shader_state);
> }
>
> static const char *
> --
> 1.8.3.1
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
More information about the Cogl
mailing list