On Fri, 17 Aug 2012, Gabriel Dos Reis wrote:
> On Fri, Aug 17, 2012 at 6:17 AM, Richard Guenther <rguent...@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.
