GNOME Bugzilla – Bug 775549
Don't start searching until the user typed a meaningful amount of characters
Last modified: 2017-03-11 23:18:27 UTC
Rule of thumb: the majority of performance problems are software trying to do too much work. I can see that with the search interface here, which lags and freezes the app for a few seconds once I start typing. That's because it tries to search and display a humongous amount of matches. Solution: don't launch the search until at least 3-4 characters have been typed, and a delay of 100 (200? 300?) miliseconds has elapsed since the last character has been typed. I have noticed that once the initial search results have started matching, continuing to type is very fast because it just filters what's already displayed...
For reference: a fast typist like me would hit around 8-10 characters per second; a moderate touch-typist (50 words per minute) will type about 4 chars par second, and a slow-poke at 24wpm would type two characters per second. So the sweet spot for the "delay after typing before search refresh" might be at 300-500 ms (I suppose beyond 500ms it would "feel" slow and you wouldn't have notable performance gains anyway).
Hi ,can you assign this bug to me please. This is my first bug and I would require some guidance while solving it.Thanks.
You don't need to be assigned to work on it, just write a patch and attach here :) Let us know if you need help.
Hi, I would like to work on this bug. Could you tell me how to set up the environment?
Hi Aaron, please follow the Newcomers guide[1] to setup your environment. [1] https://wiki.gnome.org/Newcomers
Created attachment 344103 [details] [review] gcal-search-view: Start search only if 3 or more characters are typed.
Created attachment 344336 [details] [review] Start searching when the query is greater than 2 characters.
Created attachment 344988 [details] [review] Start searching only after 4 characters and 500ms since last character.
2 or 3 seems right to me. 4 seems like too many.
Created attachment 345041 [details] [review] gcal-search-view: Start search only if 3 or more characters are typed.
@George Willian Condomitti patch seems to be better since it includes the requested delay before start search.
Review of attachment 344988 [details] [review]: The patch looks promising, but the commit message needs to be properly writted. See [1] for an example on how the commit message is expected to be. [1] https://wiki.gnome.org/Newcomers/CodeContributionWorkflow#Commit_guidelines ::: src/gcal-search-view.c @@ +721,3 @@ } +gboolean static gboolean (not public API) @@ +725,2 @@ { + GcalSearchView *view = (GcalSearchView*) user_data; This is the wrong place to set the value. Besides that, use the macro: GCAL_SEARCH_VIEW (user_data); @@ +727,3 @@ + gchar *search_query = g_strdup_printf ("(contains? \"%s\" \"%s\")", + view->field ? view->field : "summary", + view->query ? view->query : ""); The style is not correct. All the variables must be declared first, and then set. Use the GNU C Coding Style [1]. It'd be like this: """ type2 xyz; type var1, var2; xya = blabla; var1 = bla; """ var2 = blab; [1] https://developer.gnome.org/programming-guidelines/stable/c-coding-style.html.en @@ +780,3 @@ + + /* Only perform search on valid non-empty strings */ + */ '>= 3' please @@ +784,3 @@ + view->query = g_strdup (query); + view->field = g_strdup (field); +gcal_search_view_search (GcalSearchView *view, g_timeout_add() is better. ::: src/gcal-search-view.h @@ +45,3 @@ +gboolean gcal_search_view_do_search (gpointer user_data); + This is not a public function. Please remove from here.
Created attachment 345675 [details] [review] Applied code conventions and fixes as suggested by Georges Basile. Here it goes. Hope to have followed instructions correctly at this time. Thanks Georges Basile Stavracas Neto for your review.
Review of attachment 345675 [details] [review]: You should merge this patch with your previous one. The commit message improved, but it's not strictly formatted against GNOME Calendar's commit style. Check [1] for reference. [1] https://git.gnome.org/browse/gnome-calendar/log/?h=wip/gbsneto/quick-add-popover
Created attachment 346733 [details] [review] Amended commit message and merged my two patches.
Created attachment 346734 [details] [review] Amended commit message and merged my two patches. Re-sending 'cause I had forgotten '72 chars per line' limit convention in the commit message and other small nuances.
Review of attachment 344336 [details] [review]: Per discussion, this patch is not enough.
Review of attachment 345041 [details] [review]: Needs some work still ::: src/gcal-search-view.c @@ +777,3 @@ g_hash_table_remove_all (view->uuid_to_event); view->num_results = 0; + g_message ("LEn QU - %d",g_utf8_strlen (query, -1)); Stray message. @@ +779,3 @@ + g_message ("LEn QU - %d",g_utf8_strlen (query, -1)); + if (g_utf8_strlen (query, -1) < 3 && query) + gtk_label_set_label(view->title_label, "Type atleast 3 letters to start the search"); Misaligned by -2px. Also, there's a type here and the string must be translatable. @@ +781,3 @@ + gtk_label_set_label(view->title_label, "Type atleast 3 letters to start the search"); + else + gtk_label_set_label(view->title_label, "No results found"); Misaligned by -2px. Also, the string must be translatable.
Review of attachment 346734 [details] [review]: You amended the changes to the last commit, and that's good. Now you need to squash the last commit to the previous one (and have only one patch)
Created attachment 347179 [details] [review] Squashed commits into a single one.
Thanks for the patch! It works very well :)
Created attachment 347181 [details] search result not found (image) After this patch (probably, not sure), I don't get results for exact three characters. Please see the attached screenshot. Thanks
Besides the "incorrect" help text shown in the popoover in the screenshot, a way to work on this corner case could be to: 1. Not display the popover until >= 3 characters have been typed 2. Allow forced activation by hitting the "Enter" key, and/or have a 2nd timer that triggers the search after 1500 ms of inactivity