Re: [PATCH][C++] Save memory and reallocations in name-lookup

[Richard Guenther <rguenther@suse.de>]

On Fri, 17 Aug 2012, Gabriel Dos Reis wrote:

> On Fri, Aug 17, 2012 at 6:17 AM, Richard Guenther <rguenther@suse.de> wrote:
> >
> > This reduces the number of re-allocations and the amount of memory
> > wasted by store_binding.  Previously, on a cut-down testcase from
> > PR54146 we can see (--enable-gather-detailed-mem-stats -fmem-report
> > output):
> >
> > cp/name-lookup.c:5874 (store_binding)              12033504: 1.6%
> > 8564032: 2.4%          0: 0.0%    2398752:12.9%      30134
> >
> > that's the GC VEC (re-)allocation which wastes 2MB and re-allocates
> > 30134 times.  After the patch we have
> >
> > cp/name-lookup.c:5911 (store_bindings)              1243648: 0.2%
> > 352240: 0.1%          0: 0.0%     154064: 0.9%       9407
> > cp/name-lookup.c:5945 (store_class_bindings)        9643632: 1.3%
> > 120160: 0.0%          0: 0.0%     209376: 1.3%      10109
> 
> Nice saving.  As original author of the name lookup refactoring and improvement,
> I am delighted to see we can do much better.
> 
> I am however concerned with:
> 
> >   static void
> >   store_bindings (tree names, VEC(cxx_saved_binding,gc) **old_bindings)
> >   {
> > !   static VEC(tree,heap) *bindings_need_stored = NULL;
> 
> I would be more comfortable to see the cache be on per-scope
> (e.g. namespace scope) basis as opposed
> a blanket global cache stored in a global variable.

It's a "cache" only to not need a malloc/free for each store_bindings,
store_class_bindings invocation.  I've made it function local for
code cleaniness.  We're using this kind of trick elsewhere in the
compiler.

> > It seems we can end up with duplicates in the
> > names chain of store_bindings (not sure if that is intended).
>
> the chain is supposed to implement a stack, not a set.

Ah, I see.  That explains it.

Richard.