Re: [PATCH v3 1/1] Don't rewind PC for GHC generated frames

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2018-02-19 08:10 AM, Bartosz Nitka wrote:
> Hi Simon,
> 
> Thanks for taking a look at the updated version.
> 
>> I hoped there would be some comments from others.
> 
> Is there anyone we could ask directly?

I don't have anybody in particular in mind.

>> In particular, is anybody
>> able to tell if adding a call to find_pc_compunit_symtab in
>> get_frame_address_in_block is a performance concern?  How frequently is
>> get_frame_address_in_block called, and how costly is find_pc_compunit_symtab
>> to call?
> 
> This is a valid concern.
> When I originally made this change, I was focusing on making dwarf2_frame_cache
> work. dwarf2_frame_cache calls get_frame_address_in_block and then calls
> dwarf2_frame_find_quirks. dwarf2_frame_find_quirks calls
> find_pc_compunit_symtab.
> That makes me think that for this code path there should be no difference.
> 
> That's only one code path, I haven't analyzed others. If performance
> turned out to be a
> problem, then undoing the effect of get_frame_address_in_block in
> dwarf2_frame_cache
> would get me 90% of what I care about currently (unwinding).
> 
> That said, there must be a way to resolve this in a data driven way.
> Are there any pre-existing benchmarks I can run?
> Is there a scenario that I can benchmark to resolve the questions above?

You can try running the tests in testsuite/gdb.perf (see the README in there).
These tests stress GDB in some very specific ways, so they may not catch
all performance regressions in general, but it might still be a good idea to run
them anyway.  Can you do that?

>> However, please tell me if you are fine with
>> my suggestion below.
> 
> Yes, that's a good change. Thanks! If it helps move things along I'm
> happy to submit
> an updated version.

I don't think it will be necessary, I can adjust the patch before pushing it.

Simon