[PATCH] Move non-dependent gdb::observers::observable::visit_state outside template

[PATCH] Move non-dependent gdb::observers::observable::visit_state outside template

[Pedro Alves]

The other day, while looking at the symbols that end up in a GDB
index, I noticed that the gdb::observers::observable::visit_state enum
class appears a number of times:

 $ grep VISIT gdb-index-symbol-names.txt
 gdb::observers::observable<bpstat*, int>::visit_state::NOT_VISITED
 gdb::observers::observable<bpstat*, int>::visit_state::VISITED
 gdb::observers::observable<bpstat*, int>::visit_state::VISITING
 gdb::observers::observable<breakpoint*>::visit_state::NOT_VISITED
 gdb::observers::observable<breakpoint*>::visit_state::VISITED
 gdb::observers::observable<breakpoint*>::visit_state::VISITING
 gdb::observers::observable<char const*, char const*>::visit_state::NOT_VISITED
 gdb::observers::observable<char const*, char const*>::visit_state::VISITED
 gdb::observers::observable<char const*, char const*>::visit_state::VISITING
 gdb::observers::observable<char const*>::visit_state::NOT_VISITED
 gdb::observers::observable<char const*>::visit_state::VISITED
 gdb::observers::observable<char const*>::visit_state::VISITING
 gdb::observers::observable<enum_flags<user_selected_what_flag> >::visit_state::NOT_VISITED
 gdb::observers::observable<enum_flags<user_selected_what_flag> >::visit_state::VISITED
 gdb::observers::observable<enum_flags<user_selected_what_flag> >::visit_state::VISITING
 gdb::observers::observable<frame_info*, int>::visit_state::NOT_VISITED
 gdb::observers::observable<frame_info*, int>::visit_state::VISITED
 gdb::observers::observable<frame_info*, int>::visit_state::VISITING
 gdb::observers::observable<gdbarch*>::visit_state::NOT_VISITED
 gdb::observers::observable<gdbarch*>::visit_state::VISITED
 gdb::observers::observable<gdbarch*>::visit_state::VISITING
 gdb::observers::observable<gdb_signal>::visit_state::NOT_VISITED
 gdb::observers::observable<gdb_signal>::visit_state::VISITED
 gdb::observers::observable<gdb_signal>::visit_state::VISITING
 [... snip ...]

 $ grep VISIT gdb-index-symbol-names.txt | wc -l
 72

enum class visit_state is defined inside the class template
observable, but it doesn't have to be, as it does not depend on the
template parameters.  This commit moves it out, so that only one such
type exists.  This reduces the size of a -O0 -g3 build for me by
around 0.6%, like so:

 $ du -b gdb.before gdb.after
 164685280       gdb.before
 163707424       gdb.fixed

and codesize by some 0.5%.

Change-Id: I405f4ef27b8358fdd22158245b145d849b45658e
---
 gdbsupport/observable.h | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/gdbsupport/observable.h b/gdbsupport/observable.h
index a58e23dbcff..c7475e523ef 100644
--- a/gdbsupport/observable.h
+++ b/gdbsupport/observable.h
@@ -62,6 +62,22 @@ struct token
   DISABLE_COPY_AND_ASSIGN (token);
 };
 
+namespace detail
+{
+  /* Types that don't depend on any template parameter.  This saves a
+     bit of code and debug info size, compared to putting them inside
+     class observable.  */
+
+  /* Use for sorting algorithm, to indicate which observer we have
+     visited.  */
+  enum class visit_state
+  {
+    NOT_VISITED,
+    VISITING,
+    VISITED,
+  };
+}
+
 template<typename... T>
 class observable
 {
@@ -156,14 +172,6 @@ class observable
   std::vector<observer> m_observers;
   const char *m_name;
 
-  /* Use for sorting algorithm, to indicate which observer we have visited.  */
-  enum class visit_state
-  {
-    NOT_VISITED,
-    VISITING,
-    VISITED,
-  };
-
   /* Helper method for topological sort using depth-first search algorithm.
 
      Visit all dependencies of observer at INDEX in M_OBSERVERS (later referred
@@ -171,15 +179,16 @@ class observable
 
      If the observer is already visited, do nothing.  */
   void visit_for_sorting (std::vector<observer> &sorted_observers,
-                          std::vector<visit_state> &visit_states, int index)
+			  std::vector<detail::visit_state> &visit_states,
+			  int index)
   {
-    if (visit_states[index] == visit_state::VISITED)
+    if (visit_states[index] == detail::visit_state::VISITED)
       return;
 
     /* If we are already visiting this observer, it means there's a cycle.  */
-    gdb_assert (visit_states[index] != visit_state::VISITING);
+    gdb_assert (visit_states[index] != detail::visit_state::VISITING);
 
-    visit_states[index] = visit_state::VISITING;
+    visit_states[index] = detail::visit_state::VISITING;
 
     /* For each dependency of this observer...  */
     for (const token *dep : m_observers[index].dependencies)
@@ -195,7 +204,7 @@ class observable
           }
       }
 
-    visit_states[index] = visit_state::VISITED;
+    visit_states[index] = detail::visit_state::VISITED;
     sorted_observers.push_back (m_observers[index]);
   }
 
@@ -207,8 +216,8 @@ class observable
   void sort_observers ()
   {
     std::vector<observer> sorted_observers;
-    std::vector<visit_state> visit_states (m_observers.size (),
-					   visit_state::NOT_VISITED);
+    std::vector<detail::visit_state> visit_states
+      (m_observers.size (), detail::visit_state::NOT_VISITED);
 
     for (size_t i = 0; i < m_observers.size (); i++)
       visit_for_sorting (sorted_observers, visit_states, i);

base-commit: 0ee8858e7aeca5ba5f702204daad2ddd290ef229
-- 
2.36.0

Re: [PATCH] Move non-dependent gdb::observers::observable::visit_state outside template

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> enum class visit_state is defined inside the class template
Pedro> observable, but it doesn't have to be, as it does not depend on the
Pedro> template parameters.  This commit moves it out, so that only one such
Pedro> type exists.  This reduces the size of a -O0 -g3 build for me by
Pedro> around 0.6%, like so:

Looks good to me.

Tom