comparison: Add a mode to not apply filters on interface sub-graphs

This patch allows to avoid applying filters on interface diff node
sub-graphs because those filters are useful for interface impact
analysis only, which is not needed in the leaf-node report, for
instance.  When using the leaf-node report, this capability speeds up
corpus_diff::apply_filters_and_suppressions_before_reporting, hence
the functions like corpus_diff::{has_incompatible_changes,
has_net_subtype_changes} are sped up too.

That patch thus adds a --no-change-categorization option to abidiff to
avoid doing that change categorization (A.K.A applying filters).

	* doc/manuals/abidiff.rst: Document the new
	--no-change-categorization option.
	* doc/manuals/kmidiff.rst: Likewise.
	* include/abg-comparison.h
	(diff_context::perform_change_categorization): Declare new
	accessor member functions.
	* src/abg-comparison-priv.h
	(diff_context::priv::perform_change_categorization_): Add new data
	member.
	(diff_context::priv::priv): Initialize the new data member.
	* src/abg-comparison.cc
	(diff_context::perform_change_categorization): Define new accessor
	member functions.
	(corpus_diff::priv::apply_filters_and_compute_diff_stats):
	Don't apply filters on the diff node sub-graphs of interfaces when
	the user requested "no change categorization".
	* tools/abidiff.cc (options::perform_change_categorization): New
	data member.
	(options::options): Initialize the new data member.
	(display_usage): Add a help string for the new
	--no-change-categorization.
	(parse_command_line): Parse the new --no-change-categorization
	option.
	(set_diff_context_from_opts): Set the option on the diff context
	here.
	* tools/kmidiff.cc(options::perform_change_categorization): New
	data member.
	(options::options): Initialize the new data member.
	(display_usage): Add a help string for the new
	--no-change-categorization.
	(parse_command_line): Parse the new --no-change-categorization
	option.
	(set_diff_context_from_opts): Set the option on the diff context
	here.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

diff --git a/doc/manuals/abidiff.rst b/doc/manuals/abidiff.rst
index 2debc20d9a6d25cdce9df738d6d4f23aa3c6cf94..d5dc96991cfe14a6dda7b01f8ee632c4429f59aa 100644 (file)
--- a/doc/manuals/abidiff.rst
+++ b/doc/manuals/abidiff.rst
@@ -601,6 +601,18 @@ Options
 
     This option disables those optimizations.
 
+  * ``--no-change-categorization | -x``
+
+    This option disables the categorization of changes into harmless
+    and harmful changes.  Note that this categorization is a
+    pre-requisite for the filtering of changes so this option disables
+    that filtering.  The goal of this option is to speed-up the
+    execution of the program for cases where the graph of changes is
+    huge and where the user is just interested in looking at, for
+    instance, leaf node changes without caring about their possible
+    impact on interfaces.  In that case, this option would be used
+    along with the ``--leaf-changes-only`` one.
+
   * ``--ctf``
 
     When comparing binaries, extract ABI information from `CTF`_ debug

diff --git a/doc/manuals/kmidiff.rst b/doc/manuals/kmidiff.rst
index 40358b92c23b940dc84519c4adc8aa09f66a0fb5..0e0b33f1923cf33626677f39374d961d379e7c95 100644 (file)
--- a/doc/manuals/kmidiff.rst
+++ b/doc/manuals/kmidiff.rst
@@ -178,6 +178,17 @@ Options
     the :ref:`default suppression specification files
     <abidiff_default_supprs_label>` are loaded .
 
+  * ``--no-change-categorization | -x``
+
+    This option disables the categorization of changes into harmless
+    and harmful changes.  Note that this categorization is a
+    pre-requisite for the filtering of changes so this option disables
+    that filtering.  The goal of this option is to speed-up the
+    execution of the program for cases where the graph of changes is
+    huge and where the user is just interested in looking at, for
+    instance, leaf node changes without caring about their possible
+    impact on interfaces.
+
   * ``--ctf``
 
     Extract ABI information from `CTF`_ debug information, if present,

diff --git a/include/abg-comparison.h b/include/abg-comparison.h
index 506f2bb7d4fe52583e7131ccff6ad3b22923407e..2addb7acc1d80da089b1d6dc8a0632b6129c7d5f 100644 (file)
--- a/include/abg-comparison.h
+++ b/include/abg-comparison.h
@@ -761,6 +761,12 @@ public:
   void
   add_suppressions(const suppr::suppressions_type& supprs);
 
+  bool
+  perform_change_categorization() const;
+
+  void
+  perform_change_categorization(bool);
+
   void
   show_leaf_changes_only(bool f);
 

diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h
index 48a01188758bc1a1a021b3b4e55e5e0bfca26443..29d2d2aca7a4587d20a8d59bb99364496976d799 100644 (file)
--- a/src/abg-comparison-priv.h
+++ b/src/abg-comparison-priv.h
@@ -189,6 +189,7 @@ struct diff_context::priv
   corpus_diff_sptr                     corpus_diff_;
   ostream*                             default_output_stream_;
   ostream*                             error_output_stream_;
+  bool                                 perform_change_categorization_;
   bool                                 leaf_changes_only_;
   bool                                 forbid_visiting_a_node_twice_;
   bool                                 reset_visited_diffs_for_each_interface_;
@@ -219,6 +220,7 @@ struct diff_context::priv
       reporter_(),
       default_output_stream_(),
       error_output_stream_(),
+      perform_change_categorization_(true),
       leaf_changes_only_(),
       forbid_visiting_a_node_twice_(true),
       reset_visited_diffs_for_each_interface_(),

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index dc4518680abef207c17a4e7abdb472ae3e595846..886e48fdf9e8bf4e343448472e7ed4db525d3b15 100644 (file)
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -1532,6 +1532,21 @@ diff_context::add_suppressions(const suppressions_type& supprs)
                              supprs.begin(), supprs.end());
 }
 
+/// Test if it's requested to perform diff node categorization.
+///
+/// @return true iff it's requested to perform diff node
+/// categorization.
+bool
+diff_context::perform_change_categorization() const
+{return priv_->perform_change_categorization_;}
+
+/// Request change categorization or not.
+///
+/// @param f true iff change categorization is requested.
+void
+diff_context::perform_change_categorization(bool f)
+{priv_->perform_change_categorization_ = f;}
+
 /// Set the flag that indicates if the diff using this context should
 /// show only leaf changes or not.
 ///
@@ -9989,93 +10004,96 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
   diff_context_sptr ctxt = get_context();
 
   tools_utils::timer t;
-  if (get_context()->do_log())
-    {
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "applying filters to "
-               << changed_fns_.size()
-               << " changed fns ...\n";
-      t.start();
-    }
-  // Walk the changed function diff nodes to apply the categorization
-  // filters.
-  diff_sptr diff;
-  for (function_decl_diff_sptrs_type::const_iterator i =
-        changed_fns_.begin();
-       i != changed_fns_.end();
-       ++i)
+  if (ctxt->perform_change_categorization())
     {
-      diff_sptr diff = *i;
-      ctxt->maybe_apply_filters(diff);
-    }
+      if (get_context()->do_log())
+       {
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "applying filters to "
+                   << changed_fns_.size()
+                   << " changed fns ...\n";
+         t.start();
+       }
+      // Walk the changed function diff nodes to apply the categorization
+      // filters.
+      diff_sptr diff;
+      for (function_decl_diff_sptrs_type::const_iterator i =
+            changed_fns_.begin();
+          i != changed_fns_.end();
+          ++i)
+       {
+         diff_sptr diff = *i;
+         ctxt->maybe_apply_filters(diff);
+       }
 
-  if (get_context()->do_log())
-    {
-      t.stop();
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "filters to changed fn applied!:" << t << "\n";
+      if (get_context()->do_log())
+       {
+         t.stop();
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "filters to changed fn applied!:" << t << "\n";
 
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "applying filters to "
-               << sorted_changed_vars_.size()
-               << " changed vars ...\n";
-      t.start();
-    }
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "applying filters to "
+                   << sorted_changed_vars_.size()
+                   << " changed vars ...\n";
+         t.start();
+       }
 
-  // Walk the changed variable diff nodes to apply the categorization
-  // filters.
-  for (var_diff_sptrs_type::const_iterator i = sorted_changed_vars_.begin();
-       i != sorted_changed_vars_.end();
-       ++i)
-    {
-      diff_sptr diff = *i;
-      ctxt->maybe_apply_filters(diff);
-    }
+      // Walk the changed variable diff nodes to apply the categorization
+      // filters.
+      for (var_diff_sptrs_type::const_iterator i = sorted_changed_vars_.begin();
+          i != sorted_changed_vars_.end();
+          ++i)
+       {
+         diff_sptr diff = *i;
+         ctxt->maybe_apply_filters(diff);
+       }
 
-  if (get_context()->do_log())
-    {
-      t.stop();
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "filters to changed vars applied!:" << t << "\n";
+      if (get_context()->do_log())
+       {
+         t.stop();
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "filters to changed vars applied!:" << t << "\n";
 
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "applying filters to unreachable types ...\n";
-      t.start();
-    }
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "applying filters to unreachable types ...\n";
+         t.start();
+       }
 
-  // walk the changed unreachable types to apply categorization
-  // filters
-  for (diff_sptrs_type::const_iterator i =
-         changed_unreachable_types_sorted().begin();
-       i != changed_unreachable_types_sorted().end();
-       ++i)
-    {
-      diff_sptr diff = *i;
-      ctxt->maybe_apply_filters(diff);
-    }
+      // walk the changed unreachable types to apply categorization
+      // filters
+      for (diff_sptrs_type::const_iterator i =
+            changed_unreachable_types_sorted().begin();
+          i != changed_unreachable_types_sorted().end();
+          ++i)
+       {
+         diff_sptr diff = *i;
+         ctxt->maybe_apply_filters(diff);
+       }
 
-  if (get_context()->do_log())
-    {
-      t.stop();
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "filters to unreachable types applied!:" << t << "\n";
+      if (get_context()->do_log())
+       {
+         t.stop();
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "filters to unreachable types applied!:" << t << "\n";
 
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "categorizing redundant changed sub nodes ...\n";
-      t.start();
-    }
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "categorizing redundant changed sub nodes ...\n";
+         t.start();
+       }
 
-  categorize_redundant_changed_sub_nodes();
+      categorize_redundant_changed_sub_nodes();
 
-  if (get_context()->do_log())
-    {
-      t.stop();
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "redundant changed sub nodes categorized!:" << t << "\n";
+      if (get_context()->do_log())
+       {
+         t.stop();
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "redundant changed sub nodes categorized!:" << t << "\n";
 
-      std::cerr << "in apply_filters_and_compute_diff_stats:"
-               << "count changed fns ...\n";
-      t.start();
+         std::cerr << "in apply_filters_and_compute_diff_stats:"
+                   << "count changed fns ...\n";
+         t.start();
+       }
     }
 
   // Walk the changed function diff nodes to count the number of

diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index a0a670cbb2eefd26431233bdf632bded96bb063c..3613a4a39d4cb3fcc66d1198d019c3f2054a357a 100644 (file)
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -114,6 +114,7 @@ struct options
   bool                 show_impacted_interfaces;
   bool                 assume_odr_for_cplusplus;
   bool                 leverage_dwarf_factorization;
+  bool                 perform_change_categorization;
   bool                 dump_diff_tree;
   bool                 show_stats;
   bool                 do_log;
@@ -170,6 +171,7 @@ struct options
       show_impacted_interfaces(),
       assume_odr_for_cplusplus(true),
       leverage_dwarf_factorization(true),
+      perform_change_categorization(true),
       dump_diff_tree(),
       show_stats(),
       do_log()
@@ -276,6 +278,8 @@ display_usage(const string& prog_name, ostream& out)
     << " --impacted-interfaces  display interfaces impacted by leaf changes\n"
     << " --no-leverage-dwarf-factorization  do not use DWZ optimisations to "
     "speed-up the analysis of the binary\n"
+    << " --no-change-categorization | -x don't perform categorization "
+    "of changes, for speed purposes\n"
     << " --no-assume-odr-for-cplusplus  do not assume the ODR to speed-up the "
     "analysis of the binary\n"
     << " --dump-diff-tree  emit a debug dump of the internal diff tree to "
@@ -641,6 +645,9 @@ parse_command_line(int argc, char* argv[], options& opts)
        opts.show_impacted_interfaces = true;
       else if (!strcmp(argv[i], "--no-leverage-dwarf-factorization"))
        opts.leverage_dwarf_factorization = false;
+      else if (!strcmp(argv[i], "--no-change-categorization")
+              || !strcmp(argv[i], "-x"))
+       opts.perform_change_categorization = false;
       else if (!strcmp(argv[i], "--no-assume-odr-for-cplusplus"))
        opts.leverage_dwarf_factorization = false;
       else if (!strcmp(argv[i], "--dump-diff-tree"))
@@ -752,6 +759,7 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
 {
   ctxt->default_output_stream(&cout);
   ctxt->error_output_stream(&cerr);
+  ctxt->perform_change_categorization(opts.perform_change_categorization);
   ctxt->show_leaf_changes_only(opts.leaf_changes_only);
   ctxt->show_hex_values(opts.show_hexadecimal_values);
   ctxt->show_offsets_sizes_in_bits(opts.show_offsets_sizes_in_bits);

diff --git a/tools/kmidiff.cc b/tools/kmidiff.cc
index f00895a3775486504a1fa80a5e60c657f0fe3ce5..f07e4c59be6c80238d9d12fe9f24009c629022c2 100644 (file)
--- a/tools/kmidiff.cc
+++ b/tools/kmidiff.cc
@@ -56,6 +56,7 @@ struct options
   bool                 display_version;
   bool                 verbose;
   bool                 missing_operand;
+  bool                 perform_change_categorization;
   bool                 leaf_changes_only;
   bool                 show_hexadecimal_values;
   bool                 show_offsets_sizes_in_bits;
@@ -84,6 +85,7 @@ struct options
       display_version(),
       verbose(),
       missing_operand(),
+      perform_change_categorization(true),
       leaf_changes_only(true),
       show_hexadecimal_values(true),
       show_offsets_sizes_in_bits(false),
@@ -128,6 +130,8 @@ display_usage(const string& prog_name, ostream& out)
 #ifdef WITH_BTF
     << " --btf use BTF instead of DWARF in ELF files\n"
 #endif
+    << " --no-change-categorization | -x don't perform categorization "
+    "of changes, for speed purposes\n"
     << " --impacted-interfaces|-i  show interfaces impacted by ABI changes\n"
     << " --full-impact|-f  show the full impact of changes on top-most "
         "interfaces\n"
@@ -274,6 +278,9 @@ parse_command_line(int argc, char* argv[], options& opts)
       else if (!strcmp(argv[i], "--btf"))
        opts.use_btf = true;
 #endif
+      else if (!strcmp(argv[i], "--no-change-categorization")
+              || !strcmp(argv[i], "-x"))
+       opts.perform_change_categorization = false;
       else if (!strcmp(argv[i], "--impacted-interfaces")
               || !strcmp(argv[i], "-i"))
        opts.show_impacted_interfaces = true;
@@ -344,6 +351,7 @@ set_diff_context(diff_context_sptr ctxt, const options& opts)
   ctxt->show_linkage_names(false);
   ctxt->show_symbols_unreferenced_by_debug_info
     (true);
+  ctxt->perform_change_categorization(opts.perform_change_categorization);
   ctxt->show_leaf_changes_only(opts.leaf_changes_only);
   ctxt->show_impacted_interfaces(opts.show_impacted_interfaces);
   ctxt->show_hex_values(opts.show_hexadecimal_values);