Very light speed improvements

When comparing of two kernel trees showed that for huge changesets
involving highly recursive types, leaf functions used by the
categorization code dominate the performance profile.

This patch introduces some changes that help gain around 30 seconds
(out of 14 minutes) on a non-optimized build, when comparing 4.7 nd
4.8 fedora kernels (between fedora 24 and 26).

	* include/abg-comp-filter.h (has_harmless_name_change): Pass smart
	pointers by reference.
	* src/abg-comp-filter.cc (access_changed)
	(function_name_changed_but_not_symbol)
	(non_static_data_member_type_size_changed)
	(static_data_member_type_size_changed, is_compatible_change)
	(decl_name_changed, has_harmless_name_change):  Pass smart
	pointers by reference.
	* include/abg-ir.h (decl_base::set_context_rel): Take a bare
	pointer, not a smart pointer.
	* src/abg-ir.cc (decl_base::priv::context_): Make this data member
	be a naked pointer, not a smart pointer.
	(decl_base::priv::priv): Initialize it.
	(decl_base::priv::~priv): New constructor.
	(decl_base::{get_context_rel, set_scope}): Adjust.
	(class_decl::method_decl::{method_decl, set_scope}): Likewise.
	(equals): In the overload for var_decl, compare the type of the
	var first as that might be faster (to detect var_decls with
	different types) in the general case where types are
	canonicalized.

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

diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h
index 4a7be7a98160d6e1a4c7d310a38ee42b1c85a903..0fee06f12bf6710242cc198107f7281ad6a49b98 100644 (file)
--- a/include/abg-comp-filter.h
+++ b/include/abg-comp-filter.h
@@ -40,7 +40,7 @@ namespace filtering
 {
 
 bool
-has_harmless_name_change(decl_base_sptr f, decl_base_sptr s);
+has_harmless_name_change(const decl_base_sptr& f, const decl_base_sptr& s);
 
 bool
 has_virtual_mem_fn_change(const function_decl_diff* diff);

diff --git a/include/abg-ir.h b/include/abg-ir.h
index 4994477c2a183fe1b2eb20af16bf43cc400aecbb..491466383d4cf87c8ec086454944016901aa8b6d 100644 (file)
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -1112,7 +1112,7 @@ protected:
   get_context_rel();
 
   void
-  set_context_rel(context_rel_sptr c);
+  set_context_rel(context_rel *c);
 
 public:
   decl_base(const environment* e,

diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
index c9ffb4b8f085b2d8b655a4ae3dad80d9c879f18c..3f60ca7ecce265ca82d60d8a125f33f860608fca 100644 (file)
--- a/src/abg-comp-filter.cc
+++ b/src/abg-comp-filter.cc
@@ -192,7 +192,7 @@ has_type_size_change(const diff* diff)
 ///
 /// @return true iff the access specifier changed.
 static bool
-access_changed(decl_base_sptr f, decl_base_sptr s)
+access_changed(const decl_base_sptr& f, const decl_base_sptr& s)
 {
   if (!is_member_decl(f)
       || !is_member_decl(s))
@@ -219,8 +219,8 @@ access_changed(decl_base_sptr f, decl_base_sptr s)
 ///
 /// @return true if the test is positive, false otherwise.
 static bool
-function_name_changed_but_not_symbol(const function_decl_sptr f,
-                                    const function_decl_sptr s)
+function_name_changed_but_not_symbol(const function_decl_sptr& f,
+                                    const function_decl_sptr& s)
 {
   if (!f || !s)
     return false;
@@ -294,7 +294,8 @@ data_member_offset_changed(decl_base_sptr f, decl_base_sptr s)
 ///
 /// @param s the second version of the non-static data member.
 static bool
-non_static_data_member_type_size_changed(decl_base_sptr f, decl_base_sptr s)
+non_static_data_member_type_size_changed(const decl_base_sptr& f,
+                                        const decl_base_sptr& s)
 {
   if (!is_member_decl(f)
       || !is_member_decl(s))
@@ -318,7 +319,8 @@ non_static_data_member_type_size_changed(decl_base_sptr f, decl_base_sptr s)
 ///
 /// @param s the second version of the static data member.
 static bool
-static_data_member_type_size_changed(decl_base_sptr f, decl_base_sptr s)
+static_data_member_type_size_changed(const decl_base_sptr& f,
+                                    const decl_base_sptr& s)
 {
   if (!is_member_decl(f)
       || !is_member_decl(s))
@@ -343,7 +345,7 @@ static_data_member_type_size_changed(decl_base_sptr f, decl_base_sptr s)
 ///
 /// @return true if d1 and d2 are different but compatible.
 static bool
-is_compatible_change(decl_base_sptr d1, decl_base_sptr d2)
+is_compatible_change(const decl_base_sptr& d1, const decl_base_sptr& d2)
 {
   if ((d1 && d2)
       && (d1 != d2)
@@ -360,7 +362,7 @@ is_compatible_change(decl_base_sptr d1, decl_base_sptr d2)
 ///
 /// @return true if d1 and d2 have different names.
 static bool
-decl_name_changed(decl_base_sptr d1, decl_base_sptr d2)
+decl_name_changed(const decl_base_sptr& d1, const decl_base_sptr& d2)
 {
   string d1_name, d2_name;
 
@@ -383,7 +385,7 @@ decl_name_changed(decl_base_sptr d1, decl_base_sptr d2)
 ///
 /// @return true iff decl @p s represents a harmless change over @p f.
 bool
-has_harmless_name_change(decl_base_sptr f, decl_base_sptr s)
+has_harmless_name_change(const decl_base_sptr& f, const decl_base_sptr& s)
 {
   return (decl_name_changed(f, s)
          && ((is_typedef(f) && is_typedef(s))

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index da454fb0042a54356cedb442e848cc71a0dc8930..5cdf2cdae5e6fe00d31cf10350fb28894dd3ca60 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -2453,7 +2453,7 @@ struct decl_base::priv
   bool                 in_pub_sym_tab_;
   bool                 is_anonymous_;
   location             location_;
-  context_rel_sptr     context_;
+  context_rel          *context_;
   interned_string      name_;
   interned_string      qualified_parent_name_;
   // This temporary qualified name is the cache used for the qualified
@@ -2468,6 +2468,7 @@ struct decl_base::priv
   priv()
     : in_pub_sym_tab_(false),
       is_anonymous_(true),
+      context_(),
       visibility_(VISIBILITY_DEFAULT)
   {}
 
@@ -2475,6 +2476,7 @@ struct decl_base::priv
        interned_string linkage_name, visibility vis)
     : in_pub_sym_tab_(false),
       location_(locus),
+      context_(),
       name_(name),
       linkage_name_(linkage_name),
       visibility_(vis)
@@ -2487,8 +2489,14 @@ struct decl_base::priv
     : in_pub_sym_tab_(false),
       is_anonymous_(true),
       location_(l),
+      context_(),
       visibility_(VISIBILITY_DEFAULT)
   {}
+
+  ~priv()
+  {
+    delete context_;
+  }
 };// end struct decl_base::priv
 
 /// Constructor for the @ref decl_base type.
@@ -2609,17 +2617,17 @@ decl_base::set_temporary_qualified_name(const interned_string& n) const
 ///@return the context relationship for the current decl_base.
 const context_rel*
 decl_base::get_context_rel() const
-{return priv_->context_.get();}
+{return priv_->context_;}
 
 ///Getter for the context relationship.
 ///
 ///@return the context relationship for the current decl_base.
 context_rel*
 decl_base::get_context_rel()
-{return priv_->context_.get();}
+{return priv_->context_;}
 
 void
-decl_base::set_context_rel(context_rel_sptr c)
+decl_base::set_context_rel(context_rel *c)
 {priv_->context_ = c;}
 
 /// Get the hash of a decl.  If the hash hasn't been computed yet,
@@ -3008,7 +3016,7 @@ void
 decl_base::set_scope(scope_decl* scope)
 {
   if (!priv_->context_)
-    priv_->context_.reset(new context_rel(scope));
+    priv_->context_ = new context_rel(scope);
   else
     priv_->context_->set_scope(scope);
 }
@@ -10444,10 +10452,7 @@ void
 var_decl::set_scope(scope_decl* scope)
 {
   if (!get_context_rel())
-    {
-      context_rel_sptr c(new dm_context_rel(scope));
-      set_context_rel(c);
-    }
+    set_context_rel(new dm_context_rel(scope));
   else
     get_context_rel()->set_scope(scope);
 }
@@ -10475,6 +10480,18 @@ bool
 equals(const var_decl& l, const var_decl& r, change_kind* k)
 {
   bool result = true;
+
+  // First test types of variables.  This should be fast because in
+  // the general case, most types should be canonicalized.
+  if (*l.get_naked_type() != *r.get_naked_type())
+    {
+      result = false;
+      if (k)
+       *k |= SUBTYPE_CHANGE_KIND;
+      else
+       return false;
+    }
+
   // If there are underlying elf symbols for these variables,
   // compare them.  And then compare the other parts.
   const elf_symbol_sptr &s0 = l.get_symbol(), &s1 = r.get_symbol();
@@ -10542,15 +10559,6 @@ equals(const var_decl& l, const var_decl& r, change_kind* k)
        return false;
     }
 
-  if (*l.get_naked_type() != *r.get_naked_type())
-    {
-      result = false;
-      if (k)
-       *k |= SUBTYPE_CHANGE_KIND;
-      else
-       return false;
-    }
-
   return result;
 }
 
@@ -14087,8 +14095,7 @@ method_decl::method_decl(const string&          name,
     function_decl(name, static_pointer_cast<function_type>(type),
                  declared_inline, locus, linkage_name, vis, bind)
 {
-  context_rel_sptr c(new mem_fn_context_rel(0));
-  set_context_rel(c);
+  set_context_rel(new mem_fn_context_rel(0));
   set_member_function_is_const(*this, type->get_is_const());
 }
 
@@ -14122,8 +14129,7 @@ method_decl::method_decl(const string&          name,
                  (dynamic_pointer_cast<method_type>(type)),
                  declared_inline, locus, linkage_name, vis, bind)
 {
-  context_rel_sptr c(new mem_fn_context_rel(0));
-  set_context_rel(c);
+  set_context_rel(new mem_fn_context_rel(0));
 }
 
 /// A constructor for instances of method_decl.
@@ -14156,8 +14162,7 @@ method_decl::method_decl(const string&          name,
                  (dynamic_pointer_cast<method_type>(type)),
                  declared_inline, locus, linkage_name, vis, bind)
 {
-  context_rel_sptr c(new mem_fn_context_rel(0));
-  set_context_rel(c);
+  set_context_rel(new mem_fn_context_rel(0));
 }
 
 /// Set the linkage name of the method.
@@ -14196,10 +14201,7 @@ void
 method_decl::set_scope(scope_decl* scope)
 {
   if (!get_context_rel())
-    {
-      context_rel_sptr c(new mem_fn_context_rel(scope));
-      set_context_rel(c);
-    }
+    set_context_rel(new mem_fn_context_rel(scope));
   else
     get_context_rel()->set_scope(scope);
 }