pstats: Fix extremely slow constant rebuilding of chart menus in GTK impl

pandatool/src/gtk-stats/gtkStatsChartMenu.cxx

@@ -23,8 +23,77 @@ GtkStatsChartMenu(GtkStatsMonitor *monitor, int thread_index) :
   _thread_index(thread_index)
 {
   _menu = gtk_menu_new();
-  gtk_widget_show(_menu);
+
+  if (thread_index == 0) {
+    // Timeline goes first.
+    gtk_menu_shell_append(GTK_MENU_SHELL(_menu),
+      make_menu_item("Timeline", -1, GtkStatsMonitor::CT_timeline, false));
+
+    // Then the piano roll (even though it's not very useful nowadays)
+    gtk_menu_shell_append(GTK_MENU_SHELL(_menu),
+      make_menu_item("Piano Roll", -1, GtkStatsMonitor::CT_piano_roll, false));
+  }
+  else {
+    gtk_menu_shell_append(GTK_MENU_SHELL(_menu),
+      make_menu_item("Open Strip Chart", 0, GtkStatsMonitor::CT_strip_chart, false));
+    gtk_menu_shell_append(GTK_MENU_SHELL(_menu),
+      make_menu_item("Open Flame Graph", -1, GtkStatsMonitor::CT_flame_graph, false));
+  }
+
+  {
+    GtkWidget *sep = gtk_separator_menu_item_new();
+    gtk_widget_show(sep);
+    gtk_menu_shell_append(GTK_MENU_SHELL(_menu), sep);
+  }
+  _time_items_end = 3;
+
+  // Put a separator between time items and level items.
+  {
+    GtkWidget *sep = gtk_separator_menu_item_new();
+    gtk_widget_show(sep);
+    gtk_menu_shell_append(GTK_MENU_SHELL(_menu), sep);
+    _level_items_end = _time_items_end + 1;
+  }
+
+  // For the main thread menu, also some options relating to all graph windows.
+  if (thread_index == 0) {
+    GtkWidget *sep = gtk_separator_menu_item_new();
+    gtk_widget_show(sep);
+    gtk_menu_shell_append(GTK_MENU_SHELL(_menu), sep);
+
+    {
+      GtkWidget *menu_item = gtk_menu_item_new_with_label("Close All Graphs");
+      gtk_widget_show(menu_item);
+      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
+
+      g_signal_connect(G_OBJECT(menu_item), "activate",
+                       G_CALLBACK(activate_close_all),
+                       (void *)_monitor);
+    }
+
+    {
+      GtkWidget *menu_item = gtk_menu_item_new_with_label("Reopen Default Graphs");
+      gtk_widget_show(menu_item);
+      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
+
+      g_signal_connect(G_OBJECT(menu_item), "activate",
+                       G_CALLBACK(activate_reopen_default),
+                       (void *)_monitor);
+    }
+
+    {
+      GtkWidget *menu_item = gtk_menu_item_new_with_label("Save Current Layout as Default");
+      gtk_widget_show(menu_item);
+      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
+
+      g_signal_connect(G_OBJECT(menu_item), "activate",
+                       G_CALLBACK(activate_save_default),
+                       (void *)_monitor);
+    }
+  }
+
   do_update();
+  gtk_widget_show(_menu);
 }
 
 /**
@@ -91,198 +160,134 @@ do_update() {
   PStatView &view = _monitor->get_view(_thread_index);
   _last_level_index = view.get_level_index();
 
-  // First, remove all of the old entries from the menu.
-  gtk_container_foreach(GTK_CONTAINER(_menu), remove_menu_child, _menu);
-
-  // Now rebuild the menu with the new set of entries.
+  const PStatClientData *client_data = _monitor->get_client_data();
+  if (client_data->get_num_collectors() > _collector_items.size()) {
+    _collector_items.resize(client_data->get_num_collectors(), std::make_pair(nullptr, nullptr));
+  }
 
+  // The menu item(s) for the thread's frame time goes second.
+  const PStatViewLevel *view_level = view.get_top_level();
   if (_thread_index == 0) {
-    // Timeline goes first.
-    {
-      GtkStatsMonitor::MenuDef smd(_thread_index, -1, GtkStatsMonitor::CT_timeline, false);
-      const GtkStatsMonitor::MenuDef *menu_def = _monitor->add_menu(smd);
-
-      GtkWidget *menu_item = gtk_menu_item_new_with_label("Timeline");
-      gtk_widget_show(menu_item);
-      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
-
-      g_signal_connect(G_OBJECT(menu_item), "activate",
-                       G_CALLBACK(GtkStatsMonitor::menu_activate),
-                       (void *)menu_def);
+    if (add_view(_menu, view_level, false, _time_items_end)) {
+      ++_time_items_end;
+      ++_level_items_end;
     }
-
-    // And the piano roll (even though it's not very useful nowadays)
-    {
-      GtkStatsMonitor::MenuDef smd(_thread_index, -1, GtkStatsMonitor::CT_piano_roll, false);
-      const GtkStatsMonitor::MenuDef *menu_def = _monitor->add_menu(smd);
-
-      GtkWidget *menu_item = gtk_menu_item_new_with_label("Piano Roll");
-      gtk_widget_show(menu_item);
-      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
-
-      g_signal_connect(G_OBJECT(menu_item), "activate",
-                       G_CALLBACK(GtkStatsMonitor::menu_activate),
-                       (void *)menu_def);
+  } else {
+    for (int c = 0; c < view_level->get_num_children(); ++c) {
+      if (add_view(_menu, view_level->get_child(c), false, _time_items_end)) {
+        ++_time_items_end;
+        ++_level_items_end;
+      }
     }
-
-    GtkWidget *sep = gtk_separator_menu_item_new();
-    gtk_widget_show(sep);
-    gtk_menu_shell_append(GTK_MENU_SHELL(_menu), sep);
   }
 
-  // The menu item(s) for the thread's frame time goes second.
-  add_view(_menu, view.get_top_level(), false);
-
-  bool needs_separator = true;
-
   // And then the menu item(s) for each of the level values.
-  const PStatClientData *client_data = _monitor->get_client_data();
   int num_toplevel_collectors = client_data->get_num_toplevel_collectors();
   for (int tc = 0; tc < num_toplevel_collectors; tc++) {
     int collector = client_data->get_toplevel_collector(tc);
     if (client_data->has_collector(collector) &&
         client_data->get_collector_has_level(collector, _thread_index)) {
 
-      // We put a separator between the above frame collector and the first
-      // level collector.
-      if (needs_separator) {
-        GtkWidget *sep = gtk_separator_menu_item_new();
-        gtk_widget_show(sep);
-        gtk_menu_shell_append(GTK_MENU_SHELL(_menu), sep);
-
-        needs_separator = false;
-      }
-
       PStatView &level_view = _monitor->get_level_view(collector, _thread_index);
-      add_view(_menu, level_view.get_top_level(), true);
-    }
-  }
-
-  // For the main thread menu, also some options relating to all graph windows.
-  if (_thread_index == 0) {
-    GtkWidget *sep = gtk_separator_menu_item_new();
-    gtk_widget_show(sep);
-    gtk_menu_shell_append(GTK_MENU_SHELL(_menu), sep);
-
-    {
-      GtkWidget *menu_item = gtk_menu_item_new_with_label("Close All Graphs");
-      gtk_widget_show(menu_item);
-      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
-
-      g_signal_connect(G_OBJECT(menu_item), "activate",
-                       G_CALLBACK(activate_close_all),
-                       (void *)_monitor);
-    }
-
-    {
-      GtkWidget *menu_item = gtk_menu_item_new_with_label("Reopen Default Graphs");
-      gtk_widget_show(menu_item);
-      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
-
-      g_signal_connect(G_OBJECT(menu_item), "activate",
-                       G_CALLBACK(activate_reopen_default),
-                       (void *)_monitor);
-    }
-
-    {
-      GtkWidget *menu_item = gtk_menu_item_new_with_label("Save Current Layout as Default");
-      gtk_widget_show(menu_item);
-      gtk_menu_shell_append(GTK_MENU_SHELL(_menu), menu_item);
-
-      g_signal_connect(G_OBJECT(menu_item), "activate",
-                       G_CALLBACK(activate_save_default),
-                       (void *)_monitor);
+      add_view(_menu, level_view.get_top_level(), true, _level_items_end);
     }
   }
 }
 
 /**
  * Adds a new entry or entries to the menu for the indicated view and its
- * children.
+ * children.  Returns true if an item was added, false if not.
  */
-void GtkStatsChartMenu::
+bool GtkStatsChartMenu::
 add_view(GtkWidget *parent_menu, const PStatViewLevel *view_level,
-         bool show_level) {
+         bool show_level, int insert_at) {
   int collector = view_level->get_collector();
 
+  GtkWidget *&menu_item = _collector_items[collector].first;
+  GtkWidget *&menu = _collector_items[collector].second;
+
   const PStatClientData *client_data = _monitor->get_client_data();
-  std::string collector_name = client_data->get_collector_name(collector);
 
   int num_children = view_level->get_num_children();
-  if (show_level && num_children == 0) {
-    // For a level collector without children, no point in making a submenu.
+  if (menu == nullptr && num_children == 0) {
+    // For a collector without children, no point in making a submenu.  We just
+    // have the item open a strip chart directly (no point in creating a flame
+    // graph if there are no children).
+    if (menu_item != nullptr) {
+      // Already exists.
+      return false;
+    }
+
+    std::string collector_name = client_data->get_collector_name(collector);
+    menu_item = make_menu_item(
+      collector_name.c_str(), collector, GtkStatsMonitor::CT_strip_chart, show_level);
+    gtk_menu_shell_insert(GTK_MENU_SHELL(parent_menu), menu_item, insert_at);
+    return true;
+  }
+  else if (menu_item != nullptr && menu == nullptr) {
+    // Unhook the signal handler, we are creating a submenu.
     GtkStatsMonitor::MenuDef smd(_thread_index, collector, GtkStatsMonitor::CT_strip_chart, show_level);
     const GtkStatsMonitor::MenuDef *menu_def = _monitor->add_menu(smd);
 
-    GtkWidget *menu_item = gtk_menu_item_new_with_label(collector_name.c_str());
-    gtk_widget_show(menu_item);
-    gtk_menu_shell_append(GTK_MENU_SHELL(parent_menu), menu_item);
-
-    g_signal_connect(G_OBJECT(menu_item), "activate",
-                     G_CALLBACK(GtkStatsMonitor::menu_activate),
-                     (void *)menu_def);
-    return;
+    g_signal_handlers_disconnect_by_data(G_OBJECT(menu_item), (void *)menu_def);
   }
 
-  GtkWidget *menu;
-  if (!show_level && collector == 0 && num_children == 0) {
-    // Root collector without children, just add the options directly to the
-    // parent menu.
-    menu = parent_menu;
+  // Create a submenu.
+  bool added_item = false;
+  if (menu_item == nullptr) {
+    std::string collector_name = client_data->get_collector_name(collector);
+    menu_item = gtk_menu_item_new_with_label(collector_name.c_str());
+    gtk_widget_show(menu_item);
+    gtk_menu_shell_insert(GTK_MENU_SHELL(parent_menu), menu_item, insert_at);
+    added_item = true;
   }
-  else {
-    // Create a submenu.
-    GtkWidget *submenu_item = gtk_menu_item_new_with_label(collector_name.c_str());
-    gtk_widget_show(submenu_item);
-    gtk_menu_shell_append(GTK_MENU_SHELL(parent_menu), submenu_item);
 
+  if (menu == nullptr) {
     menu = gtk_menu_new();
     gtk_widget_show(menu);
-    gtk_menu_item_set_submenu(GTK_MENU_ITEM(submenu_item), menu);
-  }
+    gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu_item), menu);
 
-  {
-    GtkStatsMonitor::MenuDef smd(_thread_index, collector, GtkStatsMonitor::CT_strip_chart, show_level);
-    const GtkStatsMonitor::MenuDef *menu_def = _monitor->add_menu(smd);
-
-    GtkWidget *menu_item = gtk_menu_item_new_with_label("Open Strip Chart");
-    gtk_widget_show(menu_item);
-    gtk_menu_shell_append(GTK_MENU_SHELL(menu), menu_item);
+    gtk_menu_shell_append(GTK_MENU_SHELL(menu),
+      make_menu_item("Open Strip Chart", collector, GtkStatsMonitor::CT_strip_chart, show_level));
 
-    g_signal_connect(G_OBJECT(menu_item), "activate",
-                     G_CALLBACK(GtkStatsMonitor::menu_activate),
-                     (void *)menu_def);
-  }
+    if (!show_level) {
+      if (collector == 0) {
+        collector = -1;
+      }
 
-  if (!show_level) {
-    if (collector == 0 && num_children == 0) {
-      collector = -1;
+      gtk_menu_shell_append(GTK_MENU_SHELL(menu),
+        make_menu_item("Open Flame Graph", collector, GtkStatsMonitor::CT_flame_graph));
     }
 
-    GtkStatsMonitor::MenuDef smd(_thread_index, collector, GtkStatsMonitor::CT_flame_graph, show_level);
-    const GtkStatsMonitor::MenuDef *menu_def = _monitor->add_menu(smd);
-
-    GtkWidget *menu_item = gtk_menu_item_new_with_label("Open Flame Graph");
-    gtk_widget_show(menu_item);
-    gtk_menu_shell_append(GTK_MENU_SHELL(menu), menu_item);
-
-    g_signal_connect(G_OBJECT(menu_item), "activate",
-                     G_CALLBACK(GtkStatsMonitor::menu_activate),
-                     (void *)menu_def);
-  }
-
-  if (num_children > 0) {
     GtkWidget *sep = gtk_separator_menu_item_new();
     gtk_widget_show(sep);
     gtk_menu_shell_append(GTK_MENU_SHELL(menu), sep);
+  }
 
-    // Reverse the order since the menus are listed from the top down; we want
-    // to be visually consistent with the graphs, which list these labels from
-    // the bottom up.
-    for (int c = num_children - 1; c >= 0; c--) {
-      add_view(menu, view_level->get_child(c), show_level);
-    }
+  for (int c = 0; c < num_children; ++c) {
+    add_view(menu, view_level->get_child(c), show_level, 3 + !show_level);
   }
+
+  return added_item;
+}
+
+/**
+ *
+ */
+GtkWidget *GtkStatsChartMenu::
+make_menu_item(const char *label, int collector_index, ChartType chart_type,
+               bool show_level) {
+  GtkStatsMonitor::MenuDef smd(_thread_index, collector_index, chart_type, show_level);
+  const GtkStatsMonitor::MenuDef *menu_def = _monitor->add_menu(smd);
+
+  GtkWidget *menu_item = gtk_menu_item_new_with_label(label);
+  gtk_widget_show(menu_item);
+
+  g_signal_connect(G_OBJECT(menu_item), "activate",
+                   G_CALLBACK(GtkStatsMonitor::menu_activate),
+                   (void *)menu_def);
+
+  return menu_item;
 }
 
 /**

pandatool/src/gtk-stats/gtkStatsChartMenu.h

@@ -27,6 +27,8 @@ class PStatViewLevel;
  */
 class GtkStatsChartMenu {
 public:
+  typedef GtkStatsMonitor::ChartType ChartType;
+
   GtkStatsChartMenu(GtkStatsMonitor *monitor, int thread_index);
   ~GtkStatsChartMenu();
 
@@ -38,8 +40,10 @@ public:
   void do_update();
 
 private:
-  void add_view(GtkWidget *parent_menu, const PStatViewLevel *view_level,
-                bool show_level);
+  bool add_view(GtkWidget *parent_menu, const PStatViewLevel *view_level,
+                bool show_level, int insert_at);
+  GtkWidget *make_menu_item(const char *label, int collector_index,
+                            ChartType chart_type, bool show_level = false);
 
   static void remove_menu_child(GtkWidget *widget, gpointer data);
   static void activate_close_all(GtkWidget *widget, gpointer data);
@@ -52,6 +56,11 @@ private:
   int _last_level_index;
   GtkWidget *_menu;
   GtkWidget *_menu_item = nullptr;
+
+  // Pair of menu item, submenu
+  std::vector<std::pair<GtkWidget *, GtkWidget *> > _collector_items;
+  int _time_items_end = 0;
+  int _level_items_end = 0;
 };
 
 #endif

pandatool/src/gtk-stats/gtkStatsMonitor.cxx

@@ -163,7 +163,7 @@ new_collector(int collector_index) {
 
   // We might need to update our menus.
   for (GtkStatsChartMenu *chart_menu : _chart_menus) {
-    chart_menu->do_update();
+    chart_menu->check_update();
   }
 }