[PATCH 0/4] small XML writer changes

[PATCH 0/4] small XML writer changes

[Giuliano Procida]

Hi Dodji.

This is 2 bits of unused code removal and 2 tiny efficiency
improvements.

Cheers.

Giuliano Procida (4):
  XML writer: remove type_hasher and remaining comment
  XML writer: drop write_elf_symbols_table variable emitted_syms
  XML writer: improve slightly emission of top-level declarations
  XML writer: do not create extra temporary referenced type shared_ptr

 src/abg-writer.cc | 47 +++++++++++++++++++----------------------------
 1 file changed, 19 insertions(+), 28 deletions(-)

-- 
2.34.1.703.g22d0c6ccf7-goog

[PATCH 1/4] XML writer: remove type_hasher and remaining comment

[Giuliano Procida]

The type_hasher functor is no longer used.

	* src/abg-writer.cc (type_hasher): Remove this unused functor.
	Remove a following comment referencing it.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-writer.cc | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 24a1fd56..525f6682 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -107,18 +107,9 @@ public:
   }
 };
 
-/// A hashing functor that should be as fast as possible.
-struct type_hasher
-{
-  size_t
-  operator()(const type_base* t) const
-  {return hash_type(t);}
-}; // end struct type_hasher
-
 /// A convenience typedef for a map that associates a pointer to type
-/// to a string.  The pointer to type is hashed as fast as possible.
-typedef unordered_map<type_base*,
-		      interned_string> type_ptr_map;
+/// to a string.
+typedef unordered_map<type_base*, interned_string> type_ptr_map;
 
 // A convenience typedef for a set of type_base*.
 typedef std::unordered_set<const type_base*> type_ptr_set_type;
-- 
2.34.1.703.g22d0c6ccf7-goog

Re: [PATCH 1/4] XML writer: remove type_hasher and remaining comment

[Dodji Seketeli]

Giuliano Procida <gprocida@google.com> a écrit:

> The type_hasher functor is no longer used.
>
> 	* src/abg-writer.cc (type_hasher): Remove this unused functor.
> 	Remove a following comment referencing it.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

[...]

Cheers,

-- 
		Dodji

[PATCH 2/4] XML writer: drop write_elf_symbols_table variable emitted_syms

[Giuliano Procida]

This was introduced in commit e2d45017 and was unused then.

	* src/abg-writer.cc (write_elf_symbols_table): Drop unused
	local variable emitted_syms.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-writer.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 525f6682..53adca24 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -3158,7 +3158,6 @@ write_elf_symbols_table(const elf_symbols&	syms,
   if (syms.empty())
     return false;
 
-  unordered_map<string, bool> emitted_syms;
   for (elf_symbols::const_iterator it = syms.begin(); it != syms.end(); ++it)
     write_elf_symbol(*it, ctxt, indent);
 
-- 
2.34.1.703.g22d0c6ccf7-goog

Re: [PATCH 2/4] XML writer: drop write_elf_symbols_table variable emitted_syms

[Dodji Seketeli]

Giuliano Procida <gprocida@google.com> a écrit:

> This was introduced in commit e2d45017 and was unused then.
>
> 	* src/abg-writer.cc (write_elf_symbols_table): Drop unused
> 	local variable emitted_syms.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

[...]

Cheers,

-- 
		Dodji

[PATCH 3/4] XML writer: improve slightly emission of top-level declarations

[Giuliano Procida]

In the loop that emits declarations, the iterator already points to a
decl_base_sptr, there is no need to do another dynamic_cast. It is
also possible to simplify the loop and its conditionals.

There is no change to tests or behaviour.

	* src/abg-writer.cc (decl_is_emitted): Make decl_base_sptr
	argument a const reference.
	(write_translation_unit): Eliminate a typedef and just use a
	range-for loop without the extra dynamic cast for the non-type
	case. Use else instead of continue to make it clear there are
	only two possibilities.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-writer.cc | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 53adca24..44959a02 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -733,7 +733,7 @@ public:
   /// @return true if the decl has already been emitted, false
   /// otherwise.
   bool
-  decl_is_emitted(decl_base_sptr& decl) const
+  decl_is_emitted(const decl_base_sptr& decl) const
   {
     ABG_ASSERT(!is_type(decl));
     string repr = get_pretty_representation(decl, true);
@@ -2440,12 +2440,11 @@ write_translation_unit(write_context&		ctxt,
 				 ctxt, indent + c.get_xml_element_indent());
 
   typedef scope_decl::declarations declarations;
-  typedef declarations::const_iterator const_iterator;
-  const declarations& d = tu.get_global_scope()->get_sorted_member_decls();
+  const declarations& decls = tu.get_global_scope()->get_sorted_member_decls();
 
-  for (const_iterator i = d.begin(); i != d.end(); ++i)
+  for (const decl_base_sptr& decl : decls)
     {
-      if (type_base_sptr t = is_type(*i))
+      if (type_base_sptr t = is_type(decl))
 	{
 	  // Emit declaration-only classes that are needed. Some of
 	  // these classes can be empty.  Those beasts can be classes
@@ -2456,13 +2455,12 @@ write_translation_unit(write_context&		ctxt,
 		&& !ctxt.type_is_emitted(class_type))
 	      write_type(class_type, ctxt,
 			 indent + c.get_xml_element_indent());
-	  continue;
 	}
-
-      if (decl_base_sptr d = is_decl(*i))
-	if (ctxt.decl_is_emitted(d))
-	  continue;
-      write_decl(*i, ctxt, indent + c.get_xml_element_indent());
+      else
+	{
+	  if (!ctxt.decl_is_emitted(decl))
+	    write_decl(decl, ctxt, indent + c.get_xml_element_indent());
+	}
     }
 
   write_referenced_types(ctxt, tu, indent, is_last);
-- 
2.34.1.703.g22d0c6ccf7-goog

Re: [PATCH 3/4] XML writer: improve slightly emission of top-level declarations

[Dodji Seketeli]

Giuliano Procida <gprocida@google.com> a écrit:

> In the loop that emits declarations, the iterator already points to a
> decl_base_sptr, there is no need to do another dynamic_cast. It is
> also possible to simplify the loop and its conditionals.
>
> There is no change to tests or behaviour.
>
> 	* src/abg-writer.cc (decl_is_emitted): Make decl_base_sptr
> 	argument a const reference.
> 	(write_translation_unit): Eliminate a typedef and just use a
> 	range-for loop without the extra dynamic cast for the non-type
> 	case. Use else instead of continue to make it clear there are
> 	only two possibilities.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

[...]

Cheers,

-- 
		Dodji

[PATCH 4/4] XML writer: do not create extra temporary referenced type shared_ptr

[Giuliano Procida]

In the loop of write_referenced_types temporary shared_ptr objects are
created. In the case of a declaration, two shared_ptrs are created. It
is possible to code this so only one object is created.

This is a small optimisation with no change to tests or behaviour.

	* src/abg-writer.cc (write_referenced_types): Create temporary
	shared_ptr objects within each conditional branch instead of
	outside the conditionals and within one of the branches.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-writer.cc | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 44959a02..76c71a0c 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -2321,18 +2321,21 @@ write_referenced_types(write_context &		ctxt,
 	{
 	  // We handle types which have declarations *and* function
 	  // types here.
-	  type_base_sptr t(*i, noop_deleter());
+	  type_base* t = *i;
 	  if (!ctxt.type_is_emitted(t))
 	    {
-	      if (decl_base* d = get_type_declaration(*i))
+	      if (decl_base* d = get_type_declaration(t))
 		{
 		  decl_base_sptr decl(d, noop_deleter());
 		  write_decl_in_scope(decl, ctxt,
 				      indent + c.get_xml_element_indent());
 		}
-	      else if (function_type_sptr fn_type = is_function_type(t))
-		write_function_type(fn_type, ctxt,
-				    indent + c.get_xml_element_indent());
+	      else if (function_type* f = is_function_type(t))
+		{
+		  function_type_sptr fn_type(f, noop_deleter());
+		  write_function_type(fn_type, ctxt,
+				      indent + c.get_xml_element_indent());
+		}
 	      else
 		ABG_ASSERT_NOT_REACHED;
 	    }
-- 
2.34.1.703.g22d0c6ccf7-goog

Re: [PATCH 4/4] XML writer: do not create extra temporary referenced type shared_ptr

[Dodji Seketeli]

Giuliano Procida <gprocida@google.com> a écrit:

> In the loop of write_referenced_types temporary shared_ptr objects are
> created. In the case of a declaration, two shared_ptrs are created. It
> is possible to code this so only one object is created.
>
> This is a small optimisation with no change to tests or behaviour.
>
> 	* src/abg-writer.cc (write_referenced_types): Create temporary
> 	shared_ptr objects within each conditional branch instead of
> 	outside the conditionals and within one of the branches.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

[...]

Cheers,

-- 
		Dodji