Closed Bug 1442659 Opened 5 years ago Closed 3 years ago

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

Categories

(Toolkit :: Places, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: jrmuizel, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fxperf:p3])

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.
Whiteboard: [fxperf]
This feels like a duplicate of bug 484928.
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.
Blocks: 484928
Priority: -- → P3
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?
Flags: needinfo?(ehsan)
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.
Flags: needinfo?(ehsan)
I think this is worth at least exploring potential solutions in the near future.
Whiteboard: [fxperf] → [fxperf:p2]
Blocks: PlacesIO
See Also: → 1503481
Whiteboard: [fxperf:p2] → [fxperf:p3]
Duplicate of this bug: 1579973

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.

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

Depends on: 1593690

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

Assignee: nobody → emilio
Status: NEW → RESOLVED
Closed: 3 years ago
Depends on: 1626586
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.