1442659 - We query visited state one message per link which takes a long time (1.3s)

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

Loading https://hg.mozilla.org/try/rev/f201d300d3c65ed20c1162e2bd849d0399dd426c gives this profile: https://perfht.ml/2F9e8jP

It takes 1.3s to ask the questions and 4.3s to answer them.

Comment 1

[Mark Banner (:standard8)]

This feels like a duplicate of bug 484928.

Comment 2

[Marco Bonardo [:mak]]

Yes, I suggested some time ago that we may query that info in chunks.
We actually improved the link coloring speed last year (iirc), and thus this wasn't anymore considered a critical priority.

I'll keep this open regardless of bug 484928 for the profile.

Comment 3

[Florian Quèze [:florian]]

It feels wrong that the main thread is not processing user events for more than 4s, when the only thing in the event queue is async messages from content processes, and that none of these messages seem to take a long time to process individually.

ehsan, do you know who would be the right person to figure out if we could queue async messages with a lower priority than user events, so that user events don't get blocked by async processing?

Comment 4

[(no longer active)]

If you're talking about the red strip in the profiler UI, that's not really related to user input events.  We measure that by posting a normal event to the normal priority event queue and measure how long it takes to process it.  User input events already go to a high priority event.

But we should fix this bug, it's pretty bad regardless of the high priority event queue.

Comment 5

[Alex Thayer [:alexical] (she/her)]

I think this is worth at least exploring potential solutions in the near future.

Comment 7

[Mark Banner (:standard8)]

From bug 1579973 comment 2:

For GeckoView's history implementation, we buffer requests and wait for 250ms before sending an IPC message. There's a bug somewhere to use IdleTaskRunner instead of a timer as a further optimization. It'd be wonderful if we could make that generic, so that GeckoView and Desktop Places could use the same logic to batch visited checks.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Bug 1593690 fixes this, for the child->parent query. We may want to do the same the other way around too.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Bug 1626586 fixed this for the rest, so I think this is fixed by those two bugs.