From fd7f7383654f597d5bcb04aa02d6be954c3674b5 Mon Sep 17 00:00:00 2001 From: rdb Date: Thu, 1 Dec 2022 19:30:33 +0100 Subject: [PATCH] pstats: Fix extremely slow constant rebuilding of chart menus in GTK impl --- pandatool/src/gtk-stats/gtkStatsChartMenu.cxx | 321 +++++++++--------- pandatool/src/gtk-stats/gtkStatsChartMenu.h | 13 +- pandatool/src/gtk-stats/gtkStatsMonitor.cxx | 2 +- 3 files changed, 175 insertions(+), 161 deletions(-) diff --git a/pandatool/src/gtk-stats/gtkStatsChartMenu.cxx b/pandatool/src/gtk-stats/gtkStatsChartMenu.cxx index 569177abb4..2309c4dc36 100644 --- a/pandatool/src/gtk-stats/gtkStatsChartMenu.cxx +++ b/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. - - 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); - } - - // 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); - } - - GtkWidget *sep = gtk_separator_menu_item_new(); - gtk_widget_show(sep); - gtk_menu_shell_append(GTK_MENU_SHELL(_menu), sep); + 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. - add_view(_menu, view.get_top_level(), false); - - bool needs_separator = true; + const PStatViewLevel *view_level = view.get_top_level(); + if (_thread_index == 0) { + if (add_view(_menu, view_level, false, _time_items_end)) { + ++_time_items_end; + ++_level_items_end; + } + } 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; + } + } + } // 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. - 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; - } - - 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; - } - 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); - - menu = gtk_menu_new(); - gtk_widget_show(menu); - gtk_menu_item_set_submenu(GTK_MENU_ITEM(submenu_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); - - g_signal_connect(G_OBJECT(menu_item), "activate", - G_CALLBACK(GtkStatsMonitor::menu_activate), - (void *)menu_def); - } - - if (!show_level) { - if (collector == 0 && num_children == 0) { - collector = -1; + 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; } - GtkStatsMonitor::MenuDef smd(_thread_index, collector, GtkStatsMonitor::CT_flame_graph, show_level); + 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("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); + g_signal_handlers_disconnect_by_data(G_OBJECT(menu_item), (void *)menu_def); } - if (num_children > 0) { + // 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; + } + + if (menu == nullptr) { + menu = gtk_menu_new(); + gtk_widget_show(menu); + gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu_item), menu); + + gtk_menu_shell_append(GTK_MENU_SHELL(menu), + make_menu_item("Open Strip Chart", collector, GtkStatsMonitor::CT_strip_chart, show_level)); + + if (!show_level) { + if (collector == 0) { + collector = -1; + } + + gtk_menu_shell_append(GTK_MENU_SHELL(menu), + make_menu_item("Open Flame Graph", collector, GtkStatsMonitor::CT_flame_graph)); + } + 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; } /** diff --git a/pandatool/src/gtk-stats/gtkStatsChartMenu.h b/pandatool/src/gtk-stats/gtkStatsChartMenu.h index b9570c74c5..6a1564a0db 100644 --- a/pandatool/src/gtk-stats/gtkStatsChartMenu.h +++ b/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 > _collector_items; + int _time_items_end = 0; + int _level_items_end = 0; }; #endif diff --git a/pandatool/src/gtk-stats/gtkStatsMonitor.cxx b/pandatool/src/gtk-stats/gtkStatsMonitor.cxx index 74b11a3f89..7445022d0c 100644 --- a/pandatool/src/gtk-stats/gtkStatsMonitor.cxx +++ b/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(); } }