Freeciv

Looks Like I was a little bit optimistic ... While this fix drastically lower the number of spawn threads, it doesn't solve all the issues ...

I had to do more fixes:

replace the add_idle_callback function which looks like infinite loop for GTK4 code by animation_tick_cb function like (remove in pages.c and edit gui_main.c):

 227 static gboolean animation_tick_cb(GtkWidget *widget, GdkFrameClock *frame_clock, gpointer user_data);
...
1519   gtk_widget_add_tick_callback(map_canvas, animation_tick_cb, NULL, NULL);
...
2447 static gboolean animation_tick_cb(GtkWidget *widget,
2448                                   GdkFrameClock *frame_clock,
2449                                   gpointer user_data)
2450 {
2451     if (get_current_client_page() == PAGE_GAME) {
2452             update_animation();
2453     }
2454       return G_SOURCE_CONTINUE;
2455 }

and for the add_idle_callback(main_message_area_resize, NULL); one (in pages.c):

3605     //add_idle_callback(main_message_area_resize, NULL);                                                                                                                                                       
3606     main_message_area_resize(NULL);

 362 void main_message_area_resize(void *data)                                                                                                                                                                      
 363 {
 364   if (get_current_client_page() == PAGE_GAME) {
 365     static int old_width = 0, old_height = 0;
 366     int width = gtk_widget_get_width(GTK_WIDGET(main_message_area));
 367     int height = gtk_widget_get_height(GTK_WIDGET(main_message_area));
 368 
 369     if (width != old_width
 370         || height != old_height) {
 371       chatline_scroll_to_bottom(TRUE);
 372       old_width = width;
 373       old_height = height; 
 374     }
 375 
 376     // add_idle_callback(main_message_area_resize, NULL);
 377   }
 378 }
...
2076   /* Assumes client_state is set */
2077   timer_id = g_timeout_add(TIMER_INTERVAL, timer_callback, NULL);
2078     
2079   g_signal_connect_swapped(main_message_area, "notify::default-width",                                                                                                                                         
2080                                    G_CALLBACK(main_message_area_resize), NULL);
2081   g_signal_connect_swapped(main_message_area, "notify::default-height",
2082                                    G_CALLBACK(main_message_area_resize), NULL);
2083 }   
...

With these changes, the GUI is much more reactive and consume only a very low CPU. NOTE: I guess that the same kind of adaptation should be done for the network ping case, but that's for later :).

Hope this is clear.

Oups, forget to add this amend my initial correction by (in plrdlg.c):

 866 void real_players_dialog_update(void *unused)
 867 {
 868   GtkTreeModel *model;
 869   GtkTreeIter iter;
 870   int selected;
 871 
 872   if (NULL == players_dialog_shell) {
 873     return;
 874   }
 875 
 876   /* Save the selection. */
 877   if (gtk_tree_selection_get_selected(players_selection, &model, &iter)) {
 878     gtk_tree_model_get(model, &iter, PLR_DLG_COL_ID, &selected, -1);
 879   } else {
 880     selected = -1;
 881   }
 882 
 883   g_object_freeze_notify(G_OBJECT(players_dialog_store));
 884   g_signal_handlers_block_by_func(players_selection, G_CALLBACK(selection_callback), NULL);
 885   gtk_list_store_clear(players_dialog_store);
 886   players_iterate(pplayer) {
 887     if (!player_should_be_shown(pplayer)) {
 888       continue;
 889     }
 890     gtk_list_store_append(players_dialog_store, &iter);
 891     fill_row(players_dialog_store, &iter, pplayer);
 892     if (player_number(pplayer) == selected) {
 893       /* Restore the selection. */
 894       gtk_tree_selection_select_iter(players_selection, &iter);
 895     }
 896   } players_iterate_end;
 897 
 898   update_views();
 899   g_signal_handlers_unblock_by_func(players_selection, G_CALLBACK(selection_callback), NULL);
 900   g_object_thaw_notify(G_OBJECT(players_dialog_store));                                                                                                                                                        
 901 }

Would be better to generate a patch I guess. Let me know.

Duplicate of #1817

Ah good catch thanks. I look on all open ticket but I miss it :/ sorry.

I will submit a PR as soon as I have checked that I didn't change any "big behavior" I can see.

For example I noticed that the initial chat message is randomly displayed (it seems that there's a RC somewhere, it depends on how fast I ran the client / connect / etc ...). I saw that using the right lift of the cat box send (by design ?) to the bottom due to a resize event. Should I try to fix it ?

How deep should I investigate / take time to try fixing the gtk4 client ? (just to avoid too much time in it if it's for nothing :). While I can do some basic testing under different windows system (with my kids), I don't really have a way to build it for windows. Is there some way to beta test the changes to check that the modification I did are not breaking the windows (or mac ??) clients ?

Thanks for your feedbacks.

Caeies

see PR https://github.com/freeciv/freeciv/pull/90

BR.

The PR has 3 commits, and they really are separate things (should not be be squashed) -> we will need 3 separate tickets as well, to track each improvement separately (they may go in to different branches and/or different times)

This ticket already has discussed about the plrdlg change, so let's reserve this ticket for it.

I didn't delve too deep in to this yet, but shouldn't there still be one call to selection_callback() after all the plrdlg updates?

On a more general note: I did a test run without this patch counting times selection_callback() was triggered from real_players_dialog_update(), and it seemed to happen only a couple of times on turn change - so this doesn't seem like a major CPU hog to me, especially not a constant one. Of course, the patch still seems like a nice improvement, so we should get it in.

Marko Lindqvist wrote in #note-7:

The PR has 3 commits, and they really are separate things (should not be be squashed) -> we will need 3 separate tickets as well, to track each improvement separately (they may go in to different branches and/or different times)

"gtk4: synchronize update_animation with idle tick" -> #2221

Marko Lindqvist wrote in #note-7:

The PR has 3 commits, and they really are separate things (should not be be squashed) -> we will need 3 separate tickets as well, to track each improvement separately (they may go in to different branches and/or different times)

"gtk4: fix proposal for resize stuff" -> #2222

Hi all,

Thanks for the feedbacks. I have splitted the PR90 in 3, associated to each tickets for your review.

For the records:

I started digging the issue of 100% CPU by randomly Ctrl+C bt in gdb and it appears that it was the backtrace I get most of the time under these conditions (under a gtk-4.18 debian distro). But I agree that (as said later) it was probably not the main culprit event if it seems to have somehow reduced the number of spawn thread (but no idea why, maybe it was independant).

about :

I didn't delve too deep in to this yet, but shouldn't there still be one call to selection_callback() after all the plrdlg updates?

Sorry, no idea. I'm really not a GTK-4 expert :) nor GUI expert (I didn't do gui devs since QT3 / wxWorks 2.4/2.5).

Best regards.