Re: 'src/abg-dwarf-reader.cc:compare_dies_string_attribute_value' optimization

[Dodji Seketeli <dodji@seketeli.org>]

Hey Ben!

Man, thanks a LOT for doing this.

Please see my comments below.

Ben Woodard <woodard@redhat.com> a écrit:

> So I ran my standard test on this branch as of 3d277a9cc05873cf4aeb97273d585e0b07af917d
>
>     37 Consistency mismatches
>     /lib64/libabigail.so.0.0.0 from libabigail-2.0-2.fc36.x86_64
>     /lib64/libadwaitaqtpriv.so.1.4.1 from libadwaita-qt5-1.4.1-3.fc36.x86_64
>     /lib64/libaspell.so.15.3.1 from aspell-0.60.8-9.fc36.x86_64
>     /lib64/libboost_log.so.1.76.0 from boost-log-1.76.0-9.fc36.x86_64
>     /lib64/libclucene-core.so.2.3.3.4 from clucene-core-2.3.3.4-42.20130812.e8e3d20git.fc36.x86_64
>     /lib64/libdap.so.27.0.5 from libdap-3.20.9-2.fc36.x86_64
>     /lib64/libdcerpc-samr.so.0.0.1 from samba-libs-4.16.0-6.fc36.x86_64
>     /lib64/libdjvulibre.so.21.7.0 from djvulibre-libs-3.5.28-2.fc36.x86_64
>     /lib64/dovecot/libdovecot-storage.so.0.0.0 from dovecot-2.3.18-1.fc36.x86_64
>     /lib64/libexiv2.so.0.27.5 from exiv2-libs-0.27.5-2.fc36.x86_64
>     /lib64/libgdal.so.30.0.2 from gdal-libs-3.4.2-1.fc36.x86_64
>     /lib64/libgeos.so.3.10.2 from geos-3.10.2-4.fc36.x86_64
>     /lib64/libglibmm-2.4.so.1.3.0 from glibmm24-2.66.2-2.fc36.x86_64
>     /lib64/mozilla/plugins/gmp-gmpopenh264/system-installed/libgmpopenh264.so.2.1.1 from mozilla-openh264-2.1.1-3.fc36.x86_64
>     /lib64/libhdf5_cpp.so.200.1.0 from hdf5-1.12.1-5.fc36.x86_64
>     /lib64/libicui18n.so.67.1 from libicu67-67.1-2.fc35.x86_64
>     /lib64/libicui18n.so.69.1 from libicu-69.1-5.fc36.x86_64
>     /lib64/libicuuc.so.67.1 from libicu67-67.1-2.fc35.x86_64
>     /lib64/libicuuc.so.69.1 from libicu-69.1-5.fc36.x86_64
>     /lib64/dyninst/libinstructionAPI.so.12.0.1 from dyninst-12.0.1-3.fc36.x86_64
>     /lib64/libjavascriptcoregtk-4.0.so.18.20.4 from webkit2gtk3-jsc-2.36.0-1.fc36.x86_64
>     /lib64/libjxl.so.0.6.1 from libjxl-0.6.1-9.fc36.x86_64
>     /lib64/libkmldom.so.1.3.0 from libkml-1.3.0-37.fc36.x86_64
>     /lib64/libmusicbrainz5.so.1.0.0 from libmusicbrainz5-5.1.0-19.fc36.x86_64
>     /lib64/libOpenEXRUtil-3_1.so.30.4.1 from openexr-libs-3.1.4-1.fc36.x86_64
>     /lib64/libopenh264.so.2.1.1 from openh264-2.1.1-3.fc36.x86_64
>     /lib64/libOSMesa.so.8.0.0 from mesa-libOSMesa-22.0.1-1.fc36.x86_64
>     /lib64/libproj.so.22.2.1 from proj-8.2.1-6.fc36.x86_64
>     /lib64/libQt5WaylandClient.so.5.15.3 from qt5-qtwayland-5.15.3-1.fc36.x86_64
>     /lib64/libQt5WaylandCompositor.so.5.15.3 from qt5-qtwayland-5.15.3-1.fc36.x86_64
>     /lib64/libQt5XmlPatterns.so.5.15.3 from qt5-qtxmlpatterns-5.15.3-1.fc36.x86_64
>     /lib64/libSDL2_image-2.0.so.0.2.3 from SDL2_image-2.0.5-8.fc36.x86_64
>     /lib64/libstdc++.so.6.0.30 from libstdc++-12.0.1-0.14.fc36.x86_64
>     /lib64/libtag.so.1.18.0 from taglib-1.12-6.fc36.x86_64
>     /lib64/libreoffice/program/libuno_cppuhelpergcc3.so.3 from libreoffice-ure-7.3.2.2-1.fc36.x86_64
>     /lib64/libvtkRenderingCore.so.9.1.0 from vtk-9.1.0-6.fc36.x86_64
>     /lib64/libwebrtc_audio_processing.so.1.0.0 from webrtc-audio-processing-0.3.1-8.fc36.x86_64

Pfff :-(

> There also appears to be some sort of performance regression. I know that these are your problem children but 
> /lib64/libmozjs-68.so.0.0.0 
> /lib64/libwebkit2gtk-4.0.so.37.56.4 
> /lib64/libgs.so.9.55 
> /lib64/libperl.so.5.34.1 
> have been running solidly for more than 3 hours. I’ll let them continue all night and see if they complete or if they are in some sort of infinite loop.

I see.

> The asserts that I mentioned to you privately a few days ago have been cleared up. Thank you.
>
> One of the most common problems in that set of consistency mismatches above seems to be with C++ destructors. Here are a couple of examples:
> $ abidw --abidiff /lib64/libabigail.so.0.0.0
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>
> 2 functions with some indirect sub-type change:
>
>   [C] 'method virtual abigail::comparison::diff_node_visitor::~diff_node_visitor()' at abg-comparison.cc:11192:1 has some indirect sub-type changes:
>     parameter 0 of type 'abigail::comparison::diff_node_visitor* const' was removed
>
>   [C] 'method virtual abigail::ir::ir_node_visitor::~ir_node_visitor()' at abg-ir.cc:25400:1 has some indirect sub-type changes:
>     parameter 0 of type 'abigail::ir::ir_node_visitor* const' was removed
>
> abidw --abidiff /lib64/libaspell.so.15.3.1
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>
> 2 functions with some indirect sub-type change:
>
>   [C] 'method virtual acommon::CanHaveError::~CanHaveError()' at can_have_error.cpp:16:1 has some indirect sub-type changes:
>     parameter 0 of type 'acommon::CanHaveError* const' was removed
>
>   [C] 'method virtual aspeller::Dictionary::~Dictionary()' at data.cpp:80:1 has some indirect sub-type changes:
>     parameter 0 of type 'aspeller::Dictionary* const' was removed
>
>
> That accounts for 28 of the 37 consistency mismatches. So that is probably your highest value target to fix before moving on.

OK, following your advice, I tracked this down first and indeed this
seems related to formal parameters DIEs comparison.  I have refreshed
the 'fix-dwarf-str-cmp' branch at
https://sourceware.org/git/?p=libabigail.git;a=shortlog;h=refs/heads/fix-dwarf-str-cmp
by adding this patch to the set:
https://sourceware.org/git/?p=libabigail.git;a=commit;h=6fa52b7f12b29f9b92dd0f16b84f70d82cc7c098

I hope this fix will improve things.

> There really isn’t a pattern that I can see in the remaining failing cases. Three that you should really focus on are:
> abidw --abidiff /lib64/libmozjs-78.so.0.0.0 which is an assert()
> #5  0x00007ffff7550596 in __GI___assert_fail (assertion=0x7ffff7f2d1a3 "__abg_cond__", file=0x7ffff7f2d5c0 "../../../libabigail/src/abg-ir.cc", line=25414, function=0x7ffff7f34b08 "size_t abigail::ir::hash_as_canonical_type_or_constant(const type_base*)") at assert.c:101
>
> and 
>
> abidw --abidiff /lib64/libOSMesa.so.8.0.0 which really goes off the rails somewhere.  The indenting goes right off the right side of my jumbo terminal window. That just ain’t right.

I'll be looking at those, thanks.

[...]

Cheers,

-- 
		Dodji