[Mesa-dev] [PATCH] glsl/cse: Maintain a list of free ae_entry objects

[Ian Romanick]

On 03/27/2015 11:56 AM, Thomas Helland wrote:
> This looks good to me, and matches a solution I found
> online the other day for reducing memory allocation overhead.
> 
> Reviewed-by: Thomas Helland <thomashelland90 at gmail.com>
> 
> Did you happen to profile this? Did you identify other places that
> could need a similar treatment?

I did neither.  I was in this file doing something else, and I just
happened to notice this... and I couldn't not fix it. :)

There are probably a ton of places throughout Mesa (especially in the
compiler stack) that could use an object pool.  Like I mentioned in my
reply to Ken, we'd probably want to make a general object pool
implementation to use everywhere.

> I'm about to do a last round of micro-adjustments and profiling
> of my hash table series before sending it out, but the memory
> allocations take up such a portion of the run-time that
> profiling the table starts to get "lost in the noise".
> 
> Thomas
> 
> 2015-03-27 4:52 GMT+01:00 Ian Romanick <idr at freedesktop.org>:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> The CSE algorithm will continuously allocate new ae_entry objects.  As
>> each new basic block is exited, all of the previously allocated objects
>> are dumped.  Instead, put them in a free list and re-use them in the
>> next basic block.  Reduce, reuse, recycle!
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>>
>> ---
>>  src/glsl/opt_cse.cpp | 63 +++++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 55 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/glsl/opt_cse.cpp b/src/glsl/opt_cse.cpp
>> index 623268e..425eebc 100644
>> --- a/src/glsl/opt_cse.cpp
>> +++ b/src/glsl/opt_cse.cpp
>> @@ -63,6 +63,17 @@ public:
>>        var = NULL;
>>     }
>>
>> +   void init(ir_instruction *base_ir, ir_rvalue **val)
>> +   {
>> +      this->val = val;
>> +      this->base_ir = base_ir;
>> +      this->var = NULL;
>> +
>> +      assert(val);
>> +      assert(*val);
>> +      assert(base_ir);
>> +   }
>> +
>>     /**
>>      * The pointer to the expression that we might be able to reuse
>>      *
>> @@ -116,6 +127,18 @@ private:
>>     ir_rvalue *try_cse(ir_rvalue *rvalue);
>>     void add_to_ae(ir_rvalue **rvalue);
>>
>> +   /**
>> +    * Move all nodes from the ae list to the free list
>> +    */
>> +   void empty_ae_list();
>> +
>> +   /**
>> +    * Get and initialize a new ae_entry
>> +    *
>> +    * This will either come from the free list or be freshly allocated.
>> +    */
>> +   ae_entry *get_ae_entry(ir_rvalue **rvalue);
>> +
>>     /** List of ae_entry: The available expressions to reuse */
>>     exec_list *ae;
>>
>> @@ -126,6 +149,11 @@ private:
>>      * right.
>>      */
>>     exec_list *validate_instructions;
>> +
>> +   /**
>> +    * List of available-for-use ae_entry objects.
>> +    */
>> +   exec_list free_ae_entries;
>>  };
>>
>>  /**
>> @@ -322,6 +350,25 @@ cse_visitor::try_cse(ir_rvalue *rvalue)
>>     return NULL;
>>  }
>>
>> +void
>> +cse_visitor::empty_ae_list()
>> +{
>> +   free_ae_entries.append_list(ae);
>> +}
>> +
>> +ae_entry *
>> +cse_visitor::get_ae_entry(ir_rvalue **rvalue)
>> +{
>> +   ae_entry *entry = (ae_entry *) free_ae_entries.pop_head();
>> +   if (entry) {
>> +      entry->init(base_ir, rvalue);
>> +   } else {
>> +      entry = new(mem_ctx) ae_entry(base_ir, rvalue);
>> +   }
>> +
>> +   return entry;
>> +}
>> +
>>  /** Add the rvalue to the list of available expressions for CSE. */
>>  void
>>  cse_visitor::add_to_ae(ir_rvalue **rvalue)
>> @@ -332,7 +379,7 @@ cse_visitor::add_to_ae(ir_rvalue **rvalue)
>>        printf("\n");
>>     }
>>
>> -   ae->push_tail(new(mem_ctx) ae_entry(base_ir, rvalue));
>> +   ae->push_tail(get_ae_entry(rvalue));
>>
>>     if (debug)
>>        dump_ae(ae);
>> @@ -370,33 +417,33 @@ cse_visitor::visit_enter(ir_if *ir)
>>  {
>>     handle_rvalue(&ir->condition);
>>
>> -   ae->make_empty();
>> +   empty_ae_list();
>>     visit_list_elements(this, &ir->then_instructions);
>>
>> -   ae->make_empty();
>> +   empty_ae_list();
>>     visit_list_elements(this, &ir->else_instructions);
>>
>> -   ae->make_empty();
>> +   empty_ae_list();
>>     return visit_continue_with_parent;
>>  }
>>
>>  ir_visitor_status
>>  cse_visitor::visit_enter(ir_function_signature *ir)
>>  {
>> -   ae->make_empty();
>> +   empty_ae_list();
>>     visit_list_elements(this, &ir->body);
>>
>> -   ae->make_empty();
>> +   empty_ae_list();
>>     return visit_continue_with_parent;
>>  }
>>
>>  ir_visitor_status
>>  cse_visitor::visit_enter(ir_loop *ir)
>>  {
>> -   ae->make_empty();
>> +   empty_ae_list();
>>     visit_list_elements(this, &ir->body_instructions);
>>
>> -   ae->make_empty();
>> +   empty_ae_list();
>>     return visit_continue_with_parent;
>>  }
>>
>> --
>> 2.1.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>