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 <thomashellan...@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 <i...@freedesktop.org>: >> From: Ian Romanick <ian.d.roman...@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.roman...@intel.com> >> Cc: Jordan Justen <jordan.l.jus...@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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev