From: Dodji Seketeli Date: Wed, 9 Nov 2016 14:29:20 +0000 (+0100) Subject: Apply harmless and harmful filters in one pass X-Git-Tag: libabigail-1.0.rc6~11 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9224f40c52c2c015dd993cb1ff2d3723744ef73f;p=platform%2Fupstream%2Flibabigail.git 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 # 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 ..." to unstage) # # modified: include/abg-comp-filter.h # modified: src/abg-comp-filter.cc # modified: src/abg-comparison.cc # # Untracked files: # (use "git add ..." to include in what will be committed) # # diff.txt # prtests/ # tests/data/test-read-dwarf/libtest23.so.abi.conflict Signed-off-by: Dodji Seketeli --- 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 visit_end(diff*); }; // end class harmless_filter -class harmful_filter; +class harmless_harmful_filter; /// A convenience typedef for a shared pointer to harmful_filter. -typedef shared_ptr harmful_filter_sptr; +typedef shared_ptr harmful_harmless_filter_sptr; /// A filter that walks the diff nodes tree and tags relevant diff -/// nodes into categories considered to represent potentially harmful -/// changes. -class harmful_filter : public filter_base +/// nodes into categories considered to represent potentially harmless +/// or harmful changes. +class harmless_harmful_filter : public filter_base { virtual bool visit(diff*, bool); virtual void visit_end(diff*); -}; // end class harmful_filter +}; // end class harmless_harmful_filter } // end namespace filtering } // 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) return false; } -/// The visiting code of the harmless_filter. -/// /// Detect if the changes carried by a given diff node are deemed /// harmless and do categorize the diff node accordingly. /// @@ -776,16 +774,16 @@ has_harmful_enum_change(const diff* diff) /// /// @return true iff the traversal shall keep going after the /// completion of this function. -bool -harmless_filter::visit(diff* d, bool pre) +static bool +categorize_harmless_diff_node(diff *d, bool pre) { if (!d->has_changes()) return true; - diff_category category = NO_CHANGE_CATEGORY; - if (pre) { + diff_category category = NO_CHANGE_CATEGORY; + decl_base_sptr f = is_decl(d->first_subject()), s = is_decl(d->second_subject()); @@ -825,36 +823,8 @@ harmless_filter::visit(diff* d, bool pre) return true; } -/// Part of the visiting code of the harmless_filter. -/// -/// This function is called after the visiting of a given diff node. -/// Note that when this function is called, the visiting might not -/// have taken place *if* the node (or an equivalent node) has already -/// been visited. -/// -/// @param d the diff node that has either been visited or skipped -/// (because it has already been visited during this traversing). -void -harmless_filter::visit_end(diff* d) -{ - if (d->context()->diff_has_been_visited(d)) - { - // This node or one of its equivalent node has already been - // visited. That means at this moment, harmless_filter::visit() - // has *not* been called prior to this - // harmless_filter::visit_end() is called. In other words, only - // harmless_filter::visit_begin() and - // harmless_filter::visit_end() are called. - // - // So let's update the category of this diff node from its - // canonical node. - diff* canonical = d->get_canonical_diff(); - if (canonical) - d->add_to_local_and_inherited_categories(canonical->get_local_category()); - } -} - -/// The visiting code of the harmful_filter. +/// Detect if the changes carried by a given diff node are deemed +/// harmful and do categorize the diff node accordingly. /// /// @param d the diff node being visited. /// @@ -863,16 +833,15 @@ harmless_filter::visit_end(diff* d) /// /// @return true iff the traversal shall keep going after the /// completion of this function. -bool -harmful_filter::visit(diff* d, bool pre) +static bool +categorize_harmful_diff_node(diff *d, bool pre) { - diff_category category = NO_CHANGE_CATEGORY; - if (!d->has_changes()) return true; if (pre) { + diff_category category = NO_CHANGE_CATEGORY; decl_base_sptr f = is_decl(d->first_subject()), s = is_decl(d->second_subject()); @@ -903,7 +872,23 @@ harmful_filter::visit(diff* d, bool pre) return true; } -/// Part of the visiting code of the harmless_filter. +/// The visiting code of the harmless_harmful_filter. +/// +/// @param d the diff node being visited. +/// +/// @param pre this is true iff the node is being visited *before* the +/// children nodes of @p d. +/// +/// @return true iff the traversal shall keep going after the +/// completion of this function. +bool +harmless_harmful_filter::visit(diff* d, bool pre) +{ + return (categorize_harmless_diff_node(d, pre) + && categorize_harmful_diff_node(d, pre)); +} + +/// Part of the visiting code of the harmless_harmful_filter. /// /// This function is called after the visiting of a given diff node. /// Note that when this function is called, the visiting might not @@ -913,25 +898,23 @@ harmful_filter::visit(diff* d, bool pre) /// @param d the diff node that has either been visited or skipped /// (because it has already been visited during this traversing). void -harmful_filter::visit_end(diff* d) +harmless_harmful_filter::visit_end(diff* d) { if (d->context()->diff_has_been_visited(d)) { // This node or one of its equivalent node has already been - // visited. That means at this moment, harmful_filter::visit() - // has *not* been called prior to this - // harmful_filter::visit_end() is called. In other words, only - // harmful_filter::visit_begin() and harmful_filter::visit_end() - // are called. + // visited. That means at this moment, + // harmless_harmful_filter::visit() has *not* been called prior + // to this harmless_harmful_filter::visit_end() is called. In + // other words, only harmless_harmful_filter::visit_begin() and + // harmless_harmful_filter::visit_end() are called. // - // So let's update the category of this diff node from it's - // cnanonical node. - diff* canonical = d->get_canonical_diff(); - if (canonical) - d->add_to_local_and_inherited_categories(canonical->get_local_category()); + // So let's update the category of this diff node from its + // canonical node. + if (diff* c = d->get_canonical_diff()) + d->add_to_local_and_inherited_categories(c->get_local_category()); } } - } // end namespace filtering } // end namespace comparison } // 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() // Setup all the diff output filters we have. filtering::filter_base_sptr f; - f.reset(new filtering::harmless_filter); + f.reset(new filtering::harmless_harmful_filter); add_diff_filter(f); - f.reset(new filtering::harmful_filter); - add_diff_filter(f); + // f.reset(new filtering::harmless_filter); + // add_diff_filter(f); + + // f.reset(new filtering::harmful_filter); + // add_diff_filter(f); } /// Set the corpora that are being compared into the context, so that