1353725 - [meta] displaying the awesomebar panel could be smoother

Status: RESOLVED INCOMPLETE

(Firefox :: Address Bar, enhancement, P3)

Description

[Florian Quèze [:florian]]

Attachment: Screencast on fast macbook

The attached screencast is to be played frame by frame. It was recorded on a very fast latest generation macbook. What requires a slow frame by frame replay here will likely be user-noticeable on a low-end machine.

On the first frame where the panel is visible, we have only one item shown, and there's more whitespace before it than there should be.

On the next frame, the blue (i) icon and the settings gear icon appear.

2 frames later, the aligment issue is fixed (but moving this text to the left while it's already visible on screen causes perceived flickering), a second item (from the bookmarks) appears, and the first item gets selected.

On the next frame, a second result from bookmarks appears. 2 frames later, I have 2 more items displayed, and the globe icon appears in the first selected item.

3 more frames later, I have more items that appear. I think they are from the prepopulated history (eg. youtube, facebook, ...).

On the next frame, the favicons and the star bookmark icon appear for the 5 items that are results coming from the bookmarks.


When I typed the second letter ("l"), the first item changes to "bl - Search with Google". It doesn't have an icon. The search glass icon appears on the next frame.

2 frames later, the content of the third item is copied over the second item (so it's duplicated).

16 (!) frames later, all the following results disappear and the panel shrinks.


I'm filing this bug mostly to start investigating what should be done here, we may decide to turn this into a meta bug at some point.

I think there are a few things that we could do better here:
- display the first item at the right position immediately (likely bug 1353563)
- select the first item immediately (likely bug 1353708)
- somehow preload the icons we are sure we'll need, so that the globe icon, bookmark star, search glass and gear icon appear immediately.
- if we can't shrink the panel faster, hide the obsolete items as soon as we start displaying a new item.

I think a lot of the broken intermediate frames we display can be attributed to excessive sync layout flushes we do while displaying the panel, so I'm making this depend on bug 1353563 and bug 1353708.

Comment 1

[Marco Bonardo [:mak]]

Thank you for looking into this, there's clearly space for improvements!

(In reply to Florian Quèze [:florian] [:flo] from comment #0)
> On the next frame, the favicons and the star bookmark icon appear for the 5
> items that are results coming from the bookmarks.

Yes, favicons are read asynchronously after the results arrive, but they don't shift any content around.
I don't think it would be a win to wait for favicons contents before showing the results, so I'm not sure how we could change this.

> 2 frames later, the content of the third item is copied over the second item
> (so it's duplicated).

To avoid flickering we don't overwrite previous results until new one arrive. That happens asynchronously.
That actually also causes some privacy concerns for which we may be willing to somehow "fuzz" no more relevant results.
the other alternatives are not much better, shrinking the panel causes flicker (and that's why we added animations and delays for that), replacing results with white space doesn't look great as well.

> 16 (!) frames later, all the following results disappear and the panel
> shrinks.

Yes, there's a wanted delay here, to avoid flickering. What we had before was not better, the panel was jumping up and down like crazy.

> - select the first item immediately (likely bug 1353708)

not sure that will fix the "immediately" part, but it should improve things.

> - somehow preload the icons we are sure we'll need, so that the globe icon,
> bookmark star, search glass and gear icon appear immediately.

Ok, not possible for favicons, but possible for the layout icons. Though I wonder if preloading is the issue here, the image cache should act in most cases here already. Maybe the problem is that we take some time before setting the appropriate attribute on the result?

> - if we can't shrink the panel faster, hide the obsolete items as soon as we
> start displaying a new item.

As I said before, whatever thing we do we'll have downsides.
a. shrink the panel immediately: up/down flicker. This is exactly how we were before and we wanted to change.
b. hide obsolete items: white/text flicker
c. fuzz obsolete items (text-shadow or such?): good for privacy, not sure about looking "fast"

Comment 2

[Florian Quèze [:florian]]

(In reply to Marco Bonardo [::mak] from comment #1)
> Thank you for looking into this, there's clearly space for improvements!
> 
> (In reply to Florian Quèze [:florian] [:flo] from comment #0)
> > On the next frame, the favicons and the star bookmark icon appear for the 5
> > items that are results coming from the bookmarks.
> 
> Yes, favicons are read asynchronously after the results arrive, but they
> don't shift any content around.
> I don't think it would be a win to wait for favicons contents before showing
> the results, so I'm not sure how we could change this.

I don't think there's much to improve on that frame. Maybe the star icons could appear faster. But given that nothing shifts around, it's probably fine as is.

> > - somehow preload the icons we are sure we'll need, so that the globe icon,
> > bookmark star, search glass and gear icon appear immediately.
> 
> Ok, not possible for favicons, but possible for the layout icons. Though I
> wonder if preloading is the issue here, the image cache should act in most
> cases here already.

The video is of the first time the awesomebar panel opens after startup. So I think preloading is the issue. But that doesn't exclude some attributes being potentially set too late.

> > - if we can't shrink the panel faster, hide the obsolete items as soon as we
> > start displaying a new item.
> 
> As I said before, whatever thing we do we'll have downsides.
> a. shrink the panel immediately: up/down flicker. This is exactly how we
> were before and we wanted to change.
> b. hide obsolete items: white/text flicker
> c. fuzz obsolete items (text-shadow or such?): good for privacy, not sure
> about looking "fast"

Would it make sense to animate the opacity to 0 during exactly the delay we currently have, so that when the panel shrinks it's to remove already-blank space? UX/phlsa may have some thoughts on this too.

Comment 3

[Marco Bonardo [:mak]]

(In reply to Florian Quèze [:florian] [:flo] from comment #2)
> Would it make sense to animate the opacity to 0 during exactly the delay we
> currently have, so that when the panel shrinks it's to remove already-blank
> space? UX/phlsa may have some thoughts on this too.

On the other side, a fading wouldn't help at all with the privacy issue, unless it's really fast:
- I make a search, leave the computer unattended, someone else makes a search and he can see my previous results
- I make a presentation, at my first search everyone can see results of my last search

Yep, it's all things to discuss with UX and maybe they could have better ideas.

Comment 4

[Marco Bonardo [:mak]]

PS: Surely we could fuzz previous results AND then fade them away and that would help with both problems.

Comment 5

[Marco Bonardo [:mak]]

Florian posted these 2 profiles in bug 1353563:
- first search: https://perfht.ml/2ovtidb
- second search: https://perfht.ml/2ovpqZY

They contain a lot of interesting points to analyze.

Comment 6

[Marco Bonardo [:mak]]

not sure if worth it, there's a micro optimization we could do to listbox.xml, basically setting selectedIndex invokes selectItem, that invokes _fireOnSelect. The richlistbox version of _fireOnSelect queries .currentIndex, that does getIndexOfItem(this.currentItem) that has to iterate all the children. Since we just set the index, we already know it.
We could wrap the selectItem() call with this._selectingIndex = val; delete this._selectingIndex and use that in currentIndex, if set.
This accounts for just 3ms in the profile, likely because the awesomebar has only 10 entries, it may be larger in about:downloads, not sure if worth it.

Comment 7

[Marco Bonardo [:mak]]

Another micro-optimization:
Sqlite.jsm::bindParam is doing a concat that is not needed in most cases, it's there just for shortening code. here it accounts for 8ms.

Comment 8

[Marco Bonardo [:mak]]

autocomplete::_handleOverflow can *probably* avoid flushing, needs some testing though.

Comment 9

[Marco Bonardo [:mak]]

I'll file bugs with patches for comment 6 and comment 7. Not extreme wins, but accumulating these may be interesting since they are invoked often.

Comment 10

[Florian Quèze [:florian]]

Here is a newer profile (after the fix for bug 1353563 landed) of typing a few characters in the awesomebar: https://perfht.ml/2pnedvy

This is captured on a 2008 macbook, on OS X. This machine is now a bit slow, but still much faster than the netbook used for the profiles in comment 5. Probably comparable to current low-end hardware.

Comment 12

[Marco Bonardo [:mak]]

A profile from bkelly in bug 1365606: https://perfht.ml/2qscfdG

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Marco, in bug 1365606 comment 1 you wrote:

"we can't just do a bulk add because results of a search come ASAP, and asynchronously. Btw, I?ll just dupe this and post the profile to the other bug."

Why can't we do a react-style operation here?  Accumulate changes and then bulk apply them in the next rAF.

Also, my use case was just opening clicking the arrow next in URL bar.  I didn't type anything, so I'm not sure why search anything would happen.  (I also have search in URL bar turned off.)

Comment 14

[Marco Bonardo [:mak]]

(In reply to Ben Kelly [reviewing, but slowly][:bkelly] from comment #13)
> Why can't we do a react-style operation here?  Accumulate changes and then
> bulk apply them in the next rAF.

Yes, we could indeed do something with rAF, what I meant is that we can't wait for all the results before showing them to the user, sorry for confusion.

> Also, my use case was just opening clicking the arrow next in URL bar.  I
> didn't type anything, so I'm not sure why search anything would happen.  (I
> also have search in URL bar turned off.)

Well, we run a "search" when you press the arrow, we search the database for all typed pages in frecency reversed order. The query is async and returns results asynchronously. I said search in the more global sense, not just "search engine search", too much SQL in mind :)

Comment 15

[Florian Quèze [:florian]]

Attachment: Screencast on quantum reference hardware

This is also to be played frame by frame. Captured with an HDMI recorder on the quantum reference hardware, on today's nightly. There's still a lot of room for improvement here.

The most visible issues are:
- the first item is initially displayed too far to the right of where it should be. The next 2 items are initially displayed at the left of where they should actually be. Likely due to bug 1358792.
- when expanding the panel, sometimes only the bottom border moves, and sometimes the new area is painted black. Could this be due to bug 1356532 slowing us down to the point that we skip some frames and end up painting something unfinished?

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Clearing needinfo as Phlsa is no longer at Mozilla

Comment 17

[Marco Bonardo [:mak]]

this old photon meta bug is no more useful, we'll keep tracking the remaining dependencies