Hi Aaron,
We already discussed on irc, but just for the record.
On Mon, Feb 19, 2024 at 11:20:13PM -0500, Aaron Merey wrote:
> On Tue, Feb 13, 2024 at 8:28 AM Mark Wielaard wrote:
> >
> > > This patch's method of building the aranges list is slower than simply
> > > reading .debug_aranges. On my machine, running eu-stack on a 2.9G
> > > firefox core file takes about 8.7 seconds with this patch applied,
> > > compared to about 3.3 seconds without this patch.
> >
> > That is significant. 2.5 times slower.
> > Did you check with perf or some other profiler where exactly the extra
> > time goes. Does the new method find more aranges (and so produces
> > "better" backtraces)?
>
> I took another look at the performance and realized I made a silly
> mistake when I originally tested this. My build that was 2.5x slower
> was compiled with -O0 but I tested it against an -O2 build. Oops!
>
> With the optimization level set to -O2 in all cases, the runtime of
> 'eu-stack -s' on the original 2.9G firefox core file is only about
> 9% slower: 3.6 seconds with the patch applied compared to 3.3
> seconds without the patch.
OK, still a slowdown, but 9% is much more reasonable given we are
doing more work now. Good.
> As for the number of aranges found, there is a difference for libxul.so:
> 250435 with the patch compared to 254832 without. So 4397 fewer aranges
> are found when using the new CU iteration method. I'll dig into this and
> see if there is a problem or if it's just due to some redundancy in
> libxul's .debug_aranges. FWIW there was no change to the aranges counts
> for the other modules searched during this eu-stack firefox corefile test.
A quick way to see where the differences are is using
eu-readelf --debug-dump=decodedaranges before/after your patch.
This is opposite to what I expected. I had expected there to be more,
instead of less ranges. The difference is less than 2%. But still
interesting to know what/why.
Were there any differences in the backtraces? If not, then those
ranges might not actually have been mapping to code.
> > Might it be an idea to leave dwarf_getaranges as it is and introduce a
> > new (internal) function to get "dynamic" ranges? It looks like what
> > programs (like eu-stack and eu-addr2line) really use is dwarf_addrdie
> > and dwfl_module_addrdie. These are currently build on dwarf_getaranges,
> > but could maybe use a new interface?
>
> IMO this depends on what users expect from dwarf_getaranges. Do they
> want the exact contents of .debug_aranges (whether or not it's complete)
> or should dwarf_getaranges go beyond .debug_aranges to ensure the most
> complete results?
>
> The comment for dwarf_getaranges in libdw.h simply reads "Return list
> address ranges". Since there's no mention of .debug_aranges specifically,
> I think it's fair if dwarf_getaranges does whatever it can to ensure
> comprehensive results. In which case dwarf_getaranges should probably
> dynamically generate aranges.
You might be right that no user really cares. But as seen in the
eu-readelf code, it might also be that people expected it to map to
the ranges from .debug_aranges.
So I would be happier if we just kept the dwarf_getaranges code as
is. And just change the code in dwarf_addrdie and dwfl_module_addrdie.
We could then also introduce a new public function, dwarf_getdieranges
(?) that does the new thing. But it doesn't have to be public on the
first try as long as dwarf_addrdie and dwfl_module_addrdie work. (We
might want to change the interface of dwarf_getdieranges so it can be
"lazy" for example.)
Cheers,
Mark
