Closed Bug 1383758 Opened 7 years ago Closed 2 years ago

Opening the bookmarks menu can be slow

Categories

(Firefox :: Bookmarks & History, enhancement, P3)

Desktop
All
enhancement

Tracking

()

RESOLVED DUPLICATE of bug 1363131
Performance Impact medium
Tracking Status
firefox57 --- wontfix

People

(Reporter: dietrich, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: perf, perf:responsiveness)

I just got an email from a user with 18000 bookmarks, asking whether we're going to fix the bookmarks menu, as it takes ~2 mins for them, along with many slow-script warnings. It shouldn't be possible for it to be slow, as a primary menu.
I have ~17000 bookmarks, and I don't see this. Opening the bookmarks menu for me has always been immediate or almost immediate in both Nightly and release. In my case most of the bookmarks are in sub folders, not on the main menu as one long list.
I have loads on the main menu and see this sometimes. The cause of the problem is known - we query and load the bookmark data synchronously.
Whiteboard: [fxperf]
(In reply to Dietrich Ayala (:dietrich) from comment #2) > The cause of the > problem is known - we query and load the bookmark data synchronously. It would be nice to have a profile here to verify that the slowness is actually coming from what we expect, and that there aren't additional issues piling up on top of it.
Whiteboard: [fxperf] → [fxperf:p3]
I have a lot of profiles where I've added too many bookmarks to the menu, so this is rather easy for me to get: https://perfht.ml/2vmejIh. This is with 5000 bookmarks in the menu. I think I generated this using a script like https://gist.github.com/thomcc/892265094c8e9d15817f52a65e89695a. This uses sync's machinery for inserting records into Places (it's faster than inserting the records one by one), but doesn't require setting up sync or anything.
Severity: normal → S3

Large portions of the profile are spent in SVG parsing and painting and then destroying the prescontext for that again. I suspect what's happening is that we've got icons for some of the bookmarks that are large SVG files (e.g. if you bookmark a large SVG image itself, I expect the icon will just be the same SVG...), and we try to render all of that in the parent and it causes jank.

This would also explain why this happens for some people and not others, and suggests that although our sync places APIs may be unfortunate, they're not really the primary cause here. A reasonable fix would probably be to ensure that page-icon protocols and various other favicon handling APIs use and store normalized-to-max-128x128px icons or something like that. This would also address other issues that can arise due to favicons being loaded and painted in the parent... Marco, do we already have a separate bug for this that we can link this one to?

It would still be useful to have a more recent profile than one from 5 years ago to confirm this is still the issue.

Performance Impact: --- → medium
Flags: needinfo?(mak)
OS: Unspecified → All
Hardware: Unspecified → Desktop
Whiteboard: [fxperf:p3]

(In reply to :Gijs (he/him) from comment #7)

This would also explain why this happens for some people and not others, and suggests that although our sync places APIs may be unfortunate, they're not really the primary cause here. A reasonable fix would probably be to ensure that page-icon protocols and various other favicon handling APIs use and store normalized-to-max-128x128px icons or something like that. This would also address other issues that can arise due to favicons being loaded and painted in the parent... Marco, do we already have a separate bug for this that we can link this one to?

We do already normalize non-vector icons to specific sizes (that goes up to 192px).
We also have a maximum size limit set to 65536 bytes.

For vector icons we only respect the size limit and throw the icon away if it's larger than that. We don't try doing any resizing and I'm not sure that is possible for our image tools?

Flags: needinfo?(mak)

I'm also adding a link to https://bugzilla.mozilla.org/show_bug.cgi?id=1772264, because today sometimes we return rich icons when we should prefer non-rich ones, rich icons (like apple-touch) are usually larger.

(In reply to Marco Bonardo [:mak] from comment #8)

For vector icons we only respect the size limit and throw the icon away if it's larger than that. We don't try doing any resizing and I'm not sure that is possible for our image tools?

Not while keeping them as vectors, I imagine, but I still wonder if normalizing to a pixel version would be better. There'd be artifacts if we picked a size that then didn't fit the display size neatly, I suppose...

We are tracking the same performance issue in another issue so duping it

Status: NEW → RESOLVED
Closed: 2 years ago
Duplicate of bug: 1363131
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.