GNOME Bugzilla – Bug 547899
Limit the completion entry results to a reasonable number to avoid going through the whole model
Last modified: 2012-03-07 23:41:58 UTC
We should limit the number of calls to match_func somehow to be much more efficient. Example: - model has 4000 entries (mine has) - user starts typying slowly: 'a' - model is hit 4K times for 'a', probably 2K results are shown - user feels an ugly lockup I'm thinking of two ways: - GTK+: telling it to stop after N matches, which I have no clue how to do. GtkEntryCompletion doesn't seem to have a way. - Maintain a count of how much matches we have shown already, this is not really elegant. Maybe will attach a patch in a while. Open for discussion! Would the second option make sense?.
A quick "numerization" of my last comment: If i look for 'a': ** (epiphany:22830): DEBUG: n calls: 10890, for 10293 matches -> meaning that 10K entries are expected to be drawn in the completion popup ---> FAIL If i look for 'a' with the patch (follows): ** (epiphany:24515): DEBUG: n calls: 10890, for 021 matches -> meaning that 21 entries will be drawn in the completion popup ---> WIN And there you have an usable location bar again. Quick rationale if it's not obvious: you don't want an huge monster popup, it's not google, it's a shortcut for known addresses and vaguely recalled titles of pages. You will not go around looking for "a", you /will/ go around looking for "Insects". Perhaps we can force this limit only on smaller query strings, but I think it's reasonable to expect users to not use more than the first 20 or 40 entries shown in the popup, even 100 would be reasonable. For bigger complex searches you have the history window.
Created attachment 116662 [details] [review] [PATCH] Limit the number of matches of the completion popup. Dirty trick. --- src/ephy-location-action.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)
Ok, the patch has just a little weird bug that I'm sure is obvious but right now I'm too sleepy to find. Basically if you open a new epiphany and type 'a' you will get only 1 entry. If you delete 'a' and type 'a' again it will show MAX_MATCHES matches. Why? No clue, I'm sure it's something with the order of the if's and stuff i added. It was working on my first try. Comments welcome. See ya later.
Created attachment 118392 [details] [review] [PATCH] Limit the number of matches of the completion popup. Dirty trick. --- src/ephy-location-action.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)
#!@#!@#! damn git.
Created attachment 118393 [details] [review] [PATCH] Limit the number of matches shown in the completion popup Done by wrapping the match_func. --- lib/widgets/ephy-location-entry.c | 35 ++++++++++++++++++++++++++++++++++- src/ephy-location-action.c | 26 +++----------------------- 2 files changed, 37 insertions(+), 24 deletions(-)
IMHO, blocked by this stinky GTK+ bug: #551589. But no one has said anything yet, if you can give it a go to the test case.
*** Bug 563852 has been marked as a duplicate of this bug. ***
@Diego, is this bug still valid with the new woohoo bar?
Reality check.
Benjamin explained on IRC that the approach is wrong, since the visible_func gets called in more places than we assume. That's why we get the funky number of results. The bug still holds but needs a different fix. Benjamin suggested a custom model.
Created attachment 207059 [details] [review] ephy-location-entry: indentation fixes
Created attachment 207060 [details] [review] ephy-location-entry: limit completion results Only execute @match_func until we find a fixed number of results. This avoids processing *every* entry, *every time* the user edits the entry text.
(In reply to comment #13) > Created an attachment (id=207060) [details] [review] > ephy-location-entry: limit completion results > > Only execute @match_func until we find a fixed number of results. > This avoids processing *every* entry, *every time* the user edits the > entry text. I updated this patch so we can decide if we want this or not. Benjamin is right in that match_func is not only used by a big for() over the GtkTreeModel entries. I blame GTK+ for this, because the entry in the model should be marked only once as visible or not, not many times. That aside, the bug that we are exposing is not really noticeable: - Manuelito doesn't search for 'a' and dives into the completion popup scroll window - Susanita types "fa" to click facebook.com, which is a frequent website and hence is on top of the list due to our GtkSortModel. - Mafalda recalls a reddit from yesterday, so she types "reddit AMA webcomic", this query gives her just a few results. The match_func does not query the entire model, she is happy. The only case we would be broken is the following (considering current ephy UI): - Rico McPato types 'd', gets 100 matches, and scrolls the completion all the way to the bottom: + looking for something in particular, instead of just typying it (wtf?) + boringly browsing his history in the popup, instead of in the history window I do not think either of this reasons is, well, reasonable.
Invalid now.