[PATCH 11/13] comparison: Add a mode to not apply filters on interface sub-graphs

[Dodji Seketeli <dodji@redhat.com>]

Hello,

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>
---
 doc/manuals/abidiff.rst   |  12 +++
 doc/manuals/kmidiff.rst   |  11 +++
 include/abg-comparison.h  |   6 ++
 src/abg-comparison-priv.h |   2 +
 src/abg-comparison.cc     | 168 +++++++++++++++++++++-----------------
 tools/abidiff.cc          |   8 ++
 tools/kmidiff.cc          |   8 ++
 7 files changed, 140 insertions(+), 75 deletions(-)

diff --git a/doc/manuals/abidiff.rst b/doc/manuals/abidiff.rst
index 2debc20d..d5dc9699 100644
--- 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 40358b92..0e0b33f1 100644
--- 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 506f2bb7..2addb7ac 100644
--- 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 48a01188..29d2d2ac 100644
--- 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 dc451868..886e48fd 100644
--- 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 a0a670cb..3613a4a3 100644
--- 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 f00895a3..f07e4c59 100644
--- 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);
-- 
2.39.2



-- 
		Dodji