Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#3970 closed Enhancement (fixed)

tr_core_update() wastes CPU cycles by toggling the sort mode

Reported by: jordan Owned by: jordan
Priority: Normal Milestone: 2.20
Component: GTK+ Client Version: 2.13
Severity: Normal Keywords:
Cc:

Description

The current implementation of tr_core_update() disables sorting before updating the tree model with a foreach() call, then reenables sorting. Toggling sorting causes a lot of overhead inside of GTK+, specifically g_atomic_pointer_get(): here is a profile over a four-hour seeding period in transmission-gtk, sorted by activity. (Note: perf doesn't pick up all the symbols, but by setting a breakpoint for g_atomic_pointer_get() in gdb, I've found many of these calls come from tr_core_update()):

   13.77%  transmission-gt  libglib-2.0.so.0.2705.0 [.] g_atomic_pointer_get
            |
            --- g_atomic_pointer_get
               |          
               |--45.09%-- 0x230e570
               |          
               |--18.75%-- 0x7fffc74b38c0
               |          
               |--8.26%-- 0xb083920
               |          
               |--6.22%-- 0x7fffc74b3c10

The sorting toggling in tr_core_update() was added in r3942 -- over three years ago -- with the commit message "fix bug in gtk client that caused torrent changes to show up too slowly in the GUI." There isn't a ticket associated with that commit, but it sounds like the same problem encountered by file-list.c in ticket #3529.

In both cases, we were calling gtk_*_store_set() on a GtkTreeModel's rows from gtk_tree_model_foreach() callback. _store_set() caused the model to be resorted, which invalidated _model_foreach()'s GtkTreeIter.

In the case of tr-core, a workaround more CPU-friendly than r3942 might be to use two models: an unsorted GtkListStore that gets walked with gtk_tree_model_foreach(), and a sorted wrapper around it created with gtk_tree_model_sort_new_with_model().

Here is a profile over a two-hour seeding period in transmission-gtk using the two-model approach (sorted by activity):

    6.39%  transmission-gt  libglib-2.0.so.0.2705.0 [.] g_atomic_pointer_get
            |
            --- g_atomic_pointer_get
               |          
               |--26.70%-- 0x2409570
               |          
               |--24.71%-- 0x2542240
               |          
               |--9.99%-- 0x7fff6addbcd0

So that function's overhead has decreased from 13.77% to 6.39% in this test.

Change History (3)

comment:1 Changed 11 years ago by jordan

  • Status changed from new to assigned

comment:2 Changed 11 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

jordan * r11799 gtk/tr-core.c:

(trunk gtk) #3970 "tr_core_update() wastes CPU cycles by toggling the sort mode" -- fixed.

Long description in #3970. Split tr_core's torrent GtkTreeModel? into two models: one low-level unsorted one, and one proxy sorted one. That way we don't have to disable sorting before walking through the low-level one to sync the table's attributes with the tr_torrent and tr_stat.

comment:3 Changed 11 years ago by jordan

jordan * r11802 gtk/ (main.c tr-core.c tr-core.h):

(trunk gtk) #3970 "tr_core_update() wastes CPU cycles by toggling the sort mode" -- fix minor r11799 regression.

Clearing the model on shutdown generated a warning because it used the wrong cast. Rather than fixing the cast, add tr_core_clear() for symmetry with tr_core_load().

Note: See TracTickets for help on using tickets.