GNOME Bugzilla – Bug 771460
batch rename: don't update file list if not required
Last modified: 2021-06-18 15:50:40 UTC
When there is no change required in the list items, don't update it. How to reproduce: 1. Create several files: touch {0000..9999} 2. Select All -> Rename 3. Select 'Find and Replace Text' Result: It takes some time to get the radio button activated In the above, as the 'Entry text' in 'Find and Replace Text' is already empty, there is no need to update the items which is already populated for 'Rename using template,' but it happens, so the process gets slow.
Created attachment 335607 [details] [review] batch-rename-dialog: Check if name changed for listbox labels Update the labels in the listbox only when there is a change.
Review of attachment 335607 [details] [review]: Hey Alex thanks for looking into this! As a rule of thumb, any optimisation, if not obvious like going from O(n^2) to O(n) has to be checked with profiling and numbers. If not we really don't know where and why we have to optimise. In this case, this patch has this issue. Obviously if we don't update at all the labels, it's going to be fine. In practice, when does that happens? Batch renaming is to precisely, rename files. This case when we do nothing is a special case, rather than the regular one. What we need to optimise is the regular case, and for that this patch does not help. For that, we need numbers and find the critical paths. This is necessary for any performance issue and optimisation patch. Now let's try to see where the problem is. For that I counted the cumulative time in the functions inside this loop and the total time of that loop and the whole update_list_box: Format mode { total: 0.431736 s for replace label:0.087414 s cumulative gtk_label_set_text: 0.081784 s } Replace mode, no markup { total: 0.306157 s for replace label: 0.074522 a cumulative set_markup: 0.066431 s } Replace mode, with markup { total: 0.457975 s for replace label: 0.113585 s cumulative set_markup: 0.096477 s } OK so we have something to improve here. First, your patch helps in the case none of the modes has any string on it because it avoids the set_markup and set_text which are the biggest issues. However it doesn't help in the case one mode has some change than the other doesn't. So a more generic approach that would help in both situations is to use a GtkStack with different set of listbox per each mode. Then it will fix this case and the case that each mode has a different state. On the other hand, as you can see this loop is 1/4 of the time spent, and it's spent in functions that are not approachable by us like set_markup and set_text. Although we can take a look how to improve them, I would try to figure out where the other 3/4 of the time is spent on and fix that whenever we can, or improve it with a different approach when possible, like the GtkStack I propose now for this case. The code for this profile is attached.
Created attachment 335647 [details] the diff for this profiling case
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version of Files (nautilus), then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/nautilus/-/issues/ Thank you for your understanding and your help.