GNOME Bugzilla – Bug 763747
Repeated calculation exists when do searching
Last modified: 2016-11-08 11:41:58 UTC
Created attachment 324091 [details] [review] a patch to solve this problem When doing search, a function named "tokenize_search_string" is called by every list row of the journal list box. The function turns the search text to a token array. I think it's not necessary to do it for every list row when search text changes. Because the search text used by every list row is the same, so the function "tokenize_search_string " just need to be called once time and the token array can be saved to reduce calculation. I have solved this problem and the patch is attached below.
Review of attachment 324091 [details] [review]: Thanks for the report and the patch. Really like your idea. It can simplify the code quite a bit. You should generate the patch using "git format-patch" instead of diff. See https://wiki.gnome.org/Newcomers/CodeContributionWorkflow for more information about code contribution. Some comments are down below. ::: gnome-logs/src/gl-eventviewlist.c @@ +180,3 @@ + if (!search_text || !(*search_text)) + return NULL; search_text is never gonna be NULL, so it's not necessary to add this. @@ +444,2 @@ { + return calculate_match (entry, token_array, TRUE); Actually, we can abandon search_in_result. Call calculate_match directly. @@ +489,3 @@ if (gl_event_view_search_is_case_sensitive (view)) { + if (search_in_result (entry, priv->token_array)) return calculate_match() directly is fine. @@ +497,3 @@ { gboolean matches; + matches = calculate_match (entry, priv->token_array, FALSE); Just return calculate_match directly, as mentioned above. @@ +971,3 @@ g_clear_pointer (&priv->search_text, g_free); + if (priv->token_array) + g_ptr_array_free (priv->token_array, TRUE); Use "{ }", even thought there is only one statement in the if condition. @@ +1017,3 @@ priv->search_text = NULL; priv->boot_match = NULL; + priv->token_array = NULL; To create a pointer array, use g_ptr_array_new(). See how it was created before. @@ +1071,3 @@ + search_text_copy = g_strdup (priv->search_text); + if (priv->token_array) + g_ptr_array_free (priv->token_array, TRUE); Use "{ }" as mentioned above. @@ +1078,3 @@ gl_journal_model_fetch_more_entries (priv->journal_model, TRUE); + Invalid empty line.
thank you for your advice.
Created attachment 337920 [details] [review] Only call tokenize_search_string() once while searching Since the search functionality has changed quite a lot, I generated this new patch as attached.
Review of attachment 337920 [details] [review]: ::: src/gl-journal-model.c @@ +113,3 @@ model->journal = gl_journal_new (); model->entries = g_ptr_array_new_with_free_func (g_object_unref); + model->token_array = NULL; This is automatic, so there is no need to initialize to NULL.
Created attachment 338090 [details] [review] Improve search performance by avoiding call a function repeated for each log entry Updated patch
Created attachment 338375 [details] [review] Only call tokenize_search_string() once while searching Patch updated.
Review of attachment 338375 [details] [review]: Looks fine to me.
Comment on attachment 338375 [details] [review] Only call tokenize_search_string() once while searching Pushed to master as commit 7cf983248a47f356e2680b210e7c9eb419e55e8b.