Re: [PATCH 3/8] Code cleanup: Split dwarf2_ranges_read to a callback

[Pedro Alves <palves@redhat.com>]

On 02/19/2017 09:26 PM, Jan Kratochvil wrote:
> On Fri, 17 Feb 2017 02:19:06 +0100, Pedro Alves wrote:
>> std::function is a lot of unnecessary overhead here.  Unless you
>> manage to trigger to small-function optimization (which you won't here,
>> the callbacks are too big), this is forcing a heap allocation inside
>> std::function for every call to dwarf2_ranges_process.
> 
> These microoptimizations lead to the still broken fundamental data types and
> algorithms of GDB, having to wait for minutes to hours to print an expression
> (although usually it is not printable anyway).

"lead" here is totally a non sequitur...

> perf would show the problem if it is really significant.  It would be less
> significant with tcmalloc which GNU libc systems still do not use by default.

See urls below.  There are benchmarks all over the web.  The performance
problems with misuse of std::function are well known.  Best to nip a problem
in the bud.

> 
> 
>> Let's only use std::function for it's intended use-case of when the
>> design calls for taking, or more usually, storing, a callable whose
>> type is not knowable at compile type.
> 
> You are defining a new C++ specification here?  I have checked the code as
> I wrote it is a perfect use of std::function<> according to the ISO C++11
> specification.

The specification doesn't talk about how the classes it specifies are
meant to be used.  Only their interfaces and behaviors.  

You could do something ridiculous like only use "void*" as types of 
pointers throughout a program, and cast back to the proper dynamic type
before dereferencing them, and call it a "perfect" use according to
the spec, but that doesn't mean that that would be a be good idea.

A "perfect" use for std::function would be using it when you
need to save a list of callables in a container in a field of a
class, to call later.  E.g., it'd be a good fit to replace GDB's
observers (observer_attach..., observer_notify..., etc.)

For functions that take a callable as argument, but never store the
callable anywhere, just use it, the workaround IF you don't want
or can't use a template and you need the type-erasing of std::function, is
to wrap the callable around an std::reference_wrapper at the callee site,
using std::ref.  But that's quite inconvenient to use, because std::ref
doesn't accept rvalue references.  Meaning, #1, you have to remember
to use it, #2, you have to give the lambda's a name, like:

 auto some_callback = [&] (......) {
   .....
 };
 function_that_takes_callback_as_a_filter (foo, bar, std::ref (some_callback));

instead of the desired:

 function_that_takes_callback_as_a_filter (foo, bar, [&] (......) {
   .....
 };

So for the cases where we have a function where we:

 #1 - can't or don't want to use a template.
 #2 - want to accept any callable type (, thus need a type-erasing callable
      wrapper type like std::function).
 #3 - are sure the lifetime of the callable is as long as the
      function the callable is being passed to.
 #4 - are OK to take the callable by rvalue reference.

then what we really need is a "function_view" type.  I.e., something
like std::function that always takes a reference to a callable
instead of storing the callable inside.  Like std::string_view is to
std::string.  The standard is lacking here.

I'm not saying anything new here.  Several projects have such a thing (e.g., 
llvm::function_ref), I've seen it discussed in the std-proposals list, and
I've seen blogs about such things.  If you do the proper web searches, you'll
find explanations of all the above elsewhere.  To spare you the trouble,
I just did a search now for you, and here's what I found:

on std::function vs templates:
 http://stackoverflow.com/questions/14677997/stdfunction-vs-template

on std::function vs std::ref:
 http://blog.demofox.org/2015/02/25/avoiding-the-performance-hazzards-of-stdfunction/

Here's LLVM's llvm::function_ref:
 http://llvm.org/docs/doxygen/html/STLExtras_8h_source.html#l00060

Here's a recent blog on yet another "function_view" implementation:
 https://vittorioromeo.info/index/blog/passing_functions_to_functions.html

As it happens, I have a use for something like this in symtab.c, so
I think I'll add something like it to GDB, and avoid having to
discuss this again.

> Thanks you have noticed current GCC has missed-optimization in this case and
> that it can be workarounded by a template as you have suggested.  It leads to
> worse compilation diagnostics and we can hope this hack can be removed in the
> future.  Still I agree this workaround of GCC is worth the performance gain.

#1 - It's not a hack.

#2 - an optimization I imagine could make a difference here would be "new"
     elision, per N3664, which gcc doesn't do yet.  But in any case, it's much
     better if the type system reflects semantics, instead of relying on some
     optimization that is not guaranteed to trigger, even if the compiler
     supports it.

#3 - It's not a hack.

Thanks,
Pedro Alves