From 4e03bd47bf2371533143f7a30720f2079da8b621 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Tue, 26 Mar 2013 17:53:02 +0000 Subject: [PATCH] pipeline-hash-table: Store the hash value in the entry Calculating the hash value for a pipeline can be a bit expensive. Previously when adding a new pipeline to the hash table we would end up calculating the hash value once when checking whether the pipeline is already in the hash table and then again when adding the pipeline to the hash table. Ideally GHashTable would provide some API to add an entry with a precalculated hash value to avoid the recalculation, but seeing as it doesn't do that we can force it to avoid recalculating by storing the hash value as part of the struct we are using for the key. That way the hash func passed to GHashTable can simply return the precalculated value and we can calculate the hash outside of the GHashTable calls. Reviewed-by: Robert Bragg (cherry picked from commit 4a0460a452fd1be382fd5a92d8cfd5e0cdfd4403) --- cogl/cogl-pipeline-hash-table.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/cogl/cogl-pipeline-hash-table.c b/cogl/cogl-pipeline-hash-table.c index d3769f3c7..8df637141 100644 --- a/cogl/cogl-pipeline-hash-table.c +++ b/cogl/cogl-pipeline-hash-table.c @@ -38,6 +38,12 @@ typedef struct /* The template pipeline */ CoglPipeline *pipeline; + /* 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 + * the hash table. Instead we cache the value here and calculate it + * outside of the GHashTable. */ + unsigned int hash_value; + /* GHashTable annoyingly doesn't let us pass a user data pointer to * the hash and equal functions so to work around it we have to * store the pointer in every hash table entry. We will use this @@ -59,12 +65,8 @@ static unsigned int entry_hash (const void *data) { const CoglPipelineHashTableEntry *entry = data; - const CoglPipelineHashTable *hash = entry->hash; - return _cogl_pipeline_hash (entry->pipeline, - hash->main_state, - hash->layer_state, - 0); + return entry->hash_value; } static CoglBool @@ -114,7 +116,10 @@ _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash, dummy_entry.pipeline = key_pipeline; dummy_entry.hash = hash; - + dummy_entry.hash_value = _cogl_pipeline_hash (key_pipeline, + hash->main_state, + hash->layer_state, + 0); entry = g_hash_table_lookup (hash->table, &dummy_entry); if (entry) @@ -125,13 +130,6 @@ _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash, "unusual, so something is probably wrong!\n", hash->debug_string); - /* XXX: I wish there was a way to insert into a GHashTable with a - * pre-calculated hash value since there is a cost to calculating - * the hash of a CoglPipeline and in this case we know we have - * already called _cogl_pipeline_hash during the lookup so we could - * pass the value through to here to avoid hashing it again. - */ - /* XXX: Any keys referenced by the hash table need to remain valid * all the while that there are corresponding values, so for now we * simply make a copy of the current authority pipeline. @@ -148,6 +146,7 @@ _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash, entry = g_slice_new (CoglPipelineHashTableEntry); entry->pipeline = cogl_pipeline_copy (key_pipeline); entry->hash = hash; + entry->hash_value = dummy_entry.hash_value; g_hash_table_insert (hash->table, entry, entry);