summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDodji Seketeli <dodji@redhat.com>2016-11-09 15:29:20 +0100
committerDodji Seketeli <dodji@redhat.com>2016-11-10 14:09:50 +0100
commit9224f40c52c2c015dd993cb1ff2d3723744ef73f (patch)
tree94aa75b4af0c486e219f642dc9c89ce507c36ce9
parentAvoid stripping typedefs too much (diff)
downloadlibabigail-9224f40c52c2c015dd993cb1ff2d3723744ef73f.tar.gz
libabigail-9224f40c52c2c015dd993cb1ff2d3723744ef73f.tar.bz2
libabigail-9224f40c52c2c015dd993cb1ff2d3723744ef73f.tar.xz
Apply harmless and harmful filters in one pass
When comparing linux kernels with lots of changes, walking changes twice just to apply harmless and harmful change filters was dominating the performance profile. This patch performs the harmless and harmful filtering in one pass. This makes the overall comparison go from 15 minutes to 10 minutes when comparing a 4.7 kernel from fedora24 and a 4.8 kernel from fedora26. * include/abg-comp-filter.h (class harmless_harmful_filter): Decalre new class. (typedef harmless_harmful_filter_sptr): Declare new typedef. (class harmless_filter, class harmful_filter): Remove these class declarations. (typedef harmful_filter_sptr, harmless_filter_sptr): Remove these typedefs. * src/abg-comp-filter.cc (categorize_harmless_diff_node) (categorize_harmful_diff_node): Define new static functions. ({harmless, harmful}_filter::{visit, visit_end}): Remove these member functions. (harmless_harmful_filter::{visit, visit_end}): Define new member functions. * src/abg-comparison.cc (diff_context::diff_context): Register the new harmless_harmful_filter, and remove the premier harmless_filter and harmful_filter. Signed-off-by: Dodji Seketeli <dodji@redhat.com> # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # On branch kabidiff-dedup # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # modified: include/abg-comp-filter.h # modified: src/abg-comp-filter.cc # modified: src/abg-comparison.cc # # Untracked files: # (use "git add <file>..." to include in what will be committed) # # diff.txt # prtests/ # tests/data/test-read-dwarf/libtest23.so.abi.conflict Signed-off-by: Dodji Seketeli <dodji@redhat.com>
-rw-r--r--include/abg-comp-filter.h12
-rw-r--r--src/abg-comp-filter.cc89
-rw-r--r--src/abg-comparison.cc9
3 files changed, 48 insertions, 62 deletions
diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h
index 2a42e6ae..4a7be7a9 100644
--- a/include/abg-comp-filter.h
+++ b/include/abg-comp-filter.h
@@ -86,21 +86,21 @@ class harmless_filter : public filter_base
86 visit_end(diff*); 86 visit_end(diff*);
87}; // end class harmless_filter 87}; // end class harmless_filter
88 88
89class harmful_filter; 89class harmless_harmful_filter;
90/// A convenience typedef for a shared pointer to harmful_filter. 90/// A convenience typedef for a shared pointer to harmful_filter.
91typedef shared_ptr<harmful_filter> harmful_filter_sptr; 91typedef shared_ptr<harmless_harmful_filter> harmful_harmless_filter_sptr;
92 92
93/// A filter that walks the diff nodes tree and tags relevant diff 93/// A filter that walks the diff nodes tree and tags relevant diff
94/// nodes into categories considered to represent potentially harmful 94/// nodes into categories considered to represent potentially harmless
95/// changes. 95/// or harmful changes.
96class harmful_filter : public filter_base 96class harmless_harmful_filter : public filter_base
97{ 97{
98 virtual bool 98 virtual bool
99 visit(diff*, bool); 99 visit(diff*, bool);
100 100
101 virtual void 101 virtual void
102 visit_end(diff*); 102 visit_end(diff*);
103}; // end class harmful_filter 103}; // end class harmless_harmful_filter
104 104
105} // end namespace filtering 105} // end namespace filtering
106} // end namespace comparison 106} // end namespace comparison
diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
index 62c62139..c9ffb4b8 100644
--- a/src/abg-comp-filter.cc
+++ b/src/abg-comp-filter.cc
@@ -764,8 +764,6 @@ has_harmful_enum_change(const diff* diff)
764 return false; 764 return false;
765} 765}
766 766
767/// The visiting code of the harmless_filter.
768///
769/// Detect if the changes carried by a given diff node are deemed 767/// Detect if the changes carried by a given diff node are deemed
770/// harmless and do categorize the diff node accordingly. 768/// harmless and do categorize the diff node accordingly.
771/// 769///
@@ -776,16 +774,16 @@ has_harmful_enum_change(const diff* diff)
776/// 774///
777/// @return true iff the traversal shall keep going after the 775/// @return true iff the traversal shall keep going after the
778/// completion of this function. 776/// completion of this function.
779bool 777static bool
780harmless_filter::visit(diff* d, bool pre) 778categorize_harmless_diff_node(diff *d, bool pre)
781{ 779{
782 if (!d->has_changes()) 780 if (!d->has_changes())
783 return true; 781 return true;
784 782
785 diff_category category = NO_CHANGE_CATEGORY;
786
787 if (pre) 783 if (pre)
788 { 784 {
785 diff_category category = NO_CHANGE_CATEGORY;
786
789 decl_base_sptr f = is_decl(d->first_subject()), 787 decl_base_sptr f = is_decl(d->first_subject()),
790 s = is_decl(d->second_subject()); 788 s = is_decl(d->second_subject());
791 789
@@ -825,36 +823,8 @@ harmless_filter::visit(diff* d, bool pre)
825 return true; 823 return true;
826} 824}
827 825
828/// Part of the visiting code of the harmless_filter. 826/// Detect if the changes carried by a given diff node are deemed
829/// 827/// harmful and do categorize the diff node accordingly.
830/// This function is called after the visiting of a given diff node.
831/// Note that when this function is called, the visiting might not
832/// have taken place *if* the node (or an equivalent node) has already
833/// been visited.
834///
835/// @param d the diff node that has either been visited or skipped
836/// (because it has already been visited during this traversing).
837void
838harmless_filter::visit_end(diff* d)
839{
840 if (d->context()->diff_has_been_visited(d))
841 {
842 // This node or one of its equivalent node has already been
843 // visited. That means at this moment, harmless_filter::visit()
844 // has *not* been called prior to this
845 // harmless_filter::visit_end() is called. In other words, only
846 // harmless_filter::visit_begin() and
847 // harmless_filter::visit_end() are called.
848 //
849 // So let's update the category of this diff node from its
850 // canonical node.
851 diff* canonical = d->get_canonical_diff();
852 if (canonical)
853 d->add_to_local_and_inherited_categories(canonical->get_local_category());
854 }
855}
856
857/// The visiting code of the harmful_filter.
858/// 828///
859/// @param d the diff node being visited. 829/// @param d the diff node being visited.
860/// 830///
@@ -863,16 +833,15 @@ harmless_filter::visit_end(diff* d)
863/// 833///
864/// @return true iff the traversal shall keep going after the 834/// @return true iff the traversal shall keep going after the
865/// completion of this function. 835/// completion of this function.
866bool 836static bool
867harmful_filter::visit(diff* d, bool pre) 837categorize_harmful_diff_node(diff *d, bool pre)
868{ 838{
869 diff_category category = NO_CHANGE_CATEGORY;
870
871 if (!d->has_changes()) 839 if (!d->has_changes())
872 return true; 840 return true;
873 841
874 if (pre) 842 if (pre)
875 { 843 {
844 diff_category category = NO_CHANGE_CATEGORY;
876 decl_base_sptr f = is_decl(d->first_subject()), 845 decl_base_sptr f = is_decl(d->first_subject()),
877 s = is_decl(d->second_subject()); 846 s = is_decl(d->second_subject());
878 847
@@ -903,7 +872,23 @@ harmful_filter::visit(diff* d, bool pre)
903 return true; 872 return true;
904} 873}
905 874
906/// Part of the visiting code of the harmless_filter. 875/// The visiting code of the harmless_harmful_filter.
876///
877/// @param d the diff node being visited.
878///
879/// @param pre this is true iff the node is being visited *before* the
880/// children nodes of @p d.
881///
882/// @return true iff the traversal shall keep going after the
883/// completion of this function.
884bool
885harmless_harmful_filter::visit(diff* d, bool pre)
886{
887 return (categorize_harmless_diff_node(d, pre)
888 && categorize_harmful_diff_node(d, pre));
889}
890
891/// Part of the visiting code of the harmless_harmful_filter.
907/// 892///
908/// This function is called after the visiting of a given diff node. 893/// This function is called after the visiting of a given diff node.
909/// Note that when this function is called, the visiting might not 894/// Note that when this function is called, the visiting might not
@@ -913,25 +898,23 @@ harmful_filter::visit(diff* d, bool pre)
913/// @param d the diff node that has either been visited or skipped 898/// @param d the diff node that has either been visited or skipped
914/// (because it has already been visited during this traversing). 899/// (because it has already been visited during this traversing).
915void 900void
916harmful_filter::visit_end(diff* d) 901harmless_harmful_filter::visit_end(diff* d)
917{ 902{
918 if (d->context()->diff_has_been_visited(d)) 903 if (d->context()->diff_has_been_visited(d))
919 { 904 {
920 // This node or one of its equivalent node has already been 905 // This node or one of its equivalent node has already been
921 // visited. That means at this moment, harmful_filter::visit() 906 // visited. That means at this moment,
922 // has *not* been called prior to this 907 // harmless_harmful_filter::visit() has *not* been called prior
923 // harmful_filter::visit_end() is called. In other words, only 908 // to this harmless_harmful_filter::visit_end() is called. In
924 // harmful_filter::visit_begin() and harmful_filter::visit_end() 909 // other words, only harmless_harmful_filter::visit_begin() and
925 // are called. 910 // harmless_harmful_filter::visit_end() are called.
926 // 911 //
927 // So let's update the category of this diff node from it's 912 // So let's update the category of this diff node from its
928 // cnanonical node. 913 // canonical node.
929 diff* canonical = d->get_canonical_diff(); 914 if (diff* c = d->get_canonical_diff())
930 if (canonical) 915 d->add_to_local_and_inherited_categories(c->get_local_category());
931 d->add_to_local_and_inherited_categories(canonical->get_local_category());
932 } 916 }
933} 917}
934
935} // end namespace filtering 918} // end namespace filtering
936} // end namespace comparison 919} // end namespace comparison
937} // end namespace abigail 920} // end namespace abigail
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index f2acb25d..a599997b 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -557,11 +557,14 @@ diff_context::diff_context()
557 // Setup all the diff output filters we have. 557 // Setup all the diff output filters we have.
558 filtering::filter_base_sptr f; 558 filtering::filter_base_sptr f;
559 559
560 f.reset(new filtering::harmless_filter); 560 f.reset(new filtering::harmless_harmful_filter);
561 add_diff_filter(f); 561 add_diff_filter(f);
562 562
563 f.reset(new filtering::harmful_filter); 563 // f.reset(new filtering::harmless_filter);
564 add_diff_filter(f); 564 // add_diff_filter(f);
565
566 // f.reset(new filtering::harmful_filter);
567 // add_diff_filter(f);
565} 568}
566 569
567/// Set the corpora that are being compared into the context, so that 570/// Set the corpora that are being compared into the context, so that