The default bug view has changed. See this FAQ.

Expose frecency as a sorting order for the history sidebar (with slight query builder optimization).

RESOLVED FIXED in mozilla6

Status

()

Toolkit
Places
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

(Blocks: 1 bug, {dev-doc-complete, perf})

unspecified
mozilla6
dev-doc-complete, perf
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [not-ready for cedar][fixed-in-places])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Most of the analysis is in bug 365992. I'll just report what this bug can improve and what it cannot improve.
While this is not a regression from FX3.x, it's the second most common complain I can find regarding history (first one is history loss), and is in a release we based on speed improvements and ui responsitivity.

improves:
- most visited and last visited are instant (limited to 200 results)
- searching terms with lots of results is much faster (limited to 100 results)
- the matching algorithm is the same as the location bar (code reuse, safe, better matching)
- searches based on terms are more useful (sorted by frecency rather than by title)
- makes the move to async containers easier (less post-filtering)

WILL NOT improve:
- searches returning few results or no results at all (and their live-update)

use-case scenarios:
- using history sidebar (by most/last visited) is a pain
- leaving history sidebar open on browser close sums up the search time to the next startup time (Ts increases by seconds)
- searching history in Library or sidebar is impossible, especially for slow-typers
- User leaves a history search open in a background window and does not associate browser hangs to something he doesn't see, but directly to the browser.

Risk is somehow limited by the small amount of changes and code reuse, clearly there is some risk, but we don't plan further changes to this pieces of code so it will stay easily backed-out-able.
(Assignee)

Comment 1

6 years ago
Created attachment 508433 [details] [diff] [review]
patch v1.0

will do another trybuild from this.
Attachment #508433 - Flags: review?(dietrich)
Comment on attachment 508433 [details] [diff] [review]
patch v1.0

r+ on the code changes. however, the frecency result sorting needs a test. i'll r+ with that done.
Attachment #508433 - Flags: review?(dietrich) → review-
(Assignee)

Comment 3

6 years ago
builds
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/mak77@bonardo.net-99847d1a79f4
(Assignee)

Comment 4

6 years ago
Created attachment 508603 [details] [diff] [review]
patch v1.1

added tests
Attachment #508433 - Attachment is obsolete: true
Attachment #508603 - Flags: review?(dietrich)
Comment on attachment 508603 [details] [diff] [review]
patch v1.1

r=me.

I'm fully behind limiting the search results so that we don't hang the browser for 12 seconds, but given the late stage of the release, get approval from Johnath or Beltzner, so we get another set of eyes on this change before landing it.
Attachment #508603 - Flags: review?(dietrich) → review+
(Assignee)

Comment 6

6 years ago
Comment on attachment 508603 [details] [diff] [review]
patch v1.1

OK, asking approval, risk and gains are described in comment 0, the change mitigates the most common cases of a longstanding issue that in certain situations can make the user think the browser has awful performances (without actually blaming the right UI piece).
It also tries to be conservative in the changes.
Attachment #508603 - Flags: approval2.0?

Comment 7

6 years ago
Will this limit affect both the library and the sidebar?
I sometimes manually delete entries from the history when I feel like a lot of useless them are useless and clutter the results or when I feel the urlbar is getting slower its getting slower. When I do this I usually search for a domain, sort by visitation number and delete the ones that have 1 (or 0...) visitations and are older than a few weeks or months. And these results can be in the number of hundreds or thousands. If its limited to 100 I wouldn't be able to do this.
(Assignee)

Comment 8

6 years ago
(In reply to comment #7)
> Will this limit affect both the library and the sidebar?

yes

> I sometimes manually delete entries from the history when I feel like a lot of
> useless them are useless and clutter the results or when I feel the urlbar is
> getting slower its getting slower. When I do this I usually search for a
> domain, sort by visitation number and delete the ones that have 1 (or 0...)
> visitations and are older than a few weeks or months. And these results can be
> in the number of hundreds or thousands. If its limited to 100 I wouldn't be
> able to do this.

you can delete domains in the sidebar view "by site".
or you can delete the "older than 6 months" container in the view "by date".
you can search multiple times too, actually with this limit you would take much less time doing so, since with your usual way you have to wait seconds at each refresh, while with the change even if for your specific use case requires multiple searches they will be much faster.
Btw, you don't need to to this manual cleanup, we expire entries when we reach our natural limit, if you think the limit is too large you can reduce it (see http://blog.bonardo.net/2010/01/20/places-got-async-expiration)
Comment on attachment 508603 [details] [diff] [review]
patch v1.1

Too much changing here to be comfortable about taking this at this point
Attachment #508603 - Flags: approval2.0? → approval2.0-
(Assignee)

Comment 10

6 years ago
please don't push this, I want to revise it a bit since there is more time available to make it more awesome.
(Assignee)

Updated

6 years ago
Whiteboard: don't push

Updated

6 years ago
Whiteboard: don't push → not-ready
(Assignee)

Comment 11

6 years ago
Created attachment 525777 [details] [diff] [review]
patch v1.2

I'm slightly reducing scope of the bug, I'm removing from the patch all the numeric limits.
The limited views were a solution I was mostly suggesting for FF4, but now that pressure is solved and I only want to take the good part of the patch without introducing behavioral differences. I actually got new ideas to improve performances in the meanwhile.

Well, there is still one difference, the sidebar, when not sorted by last or most visited, will be sorted by frecency, that should be a better ordering to find stuff, it will act like an infinite awesomebar (with some less awesomeness though).

From various testing the limit was also effective only in some case, while when searching for rare terms we were still going the slow path. This patch goes toward the right direction allowing to reduce some of the post-filtering work, thus pushing it since the changes are not additive from the r+.
Attachment #508603 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Summary: Limit history searches in the UI to a reasonable amount → Expose frecency as a sorting order for the history sidebar (with slight query builder optimization).
(Assignee)

Comment 12

6 years ago
http://hg.mozilla.org/projects/places/rev/24f177c8b6f6
Whiteboard: not-ready → [not-ready for cedar][fixed-in-places]
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/mozilla-central/rev/24f177c8b6f6
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
(Assignee)

Comment 14

6 years ago
dev-doc-needed for the additional SORT_BY_FRECENCY_* options
Keywords: dev-doc-needed
Documented:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsINavHistoryQueryOptions#Sorting_methods

Also fixed a couple of the other constants, which had the wrong numbers, and removed two that apparently were documented at one point then removed before shipping.

Also mentioned on Firefox 6 for developers.
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Updated

6 years ago
Blocks: 411591
(Assignee)

Updated

6 years ago
Depends on: 658242

Updated

6 years ago
Depends on: 661767
You need to log in before you can comment on or make changes to this bug.