Closed Bug 1427607 Opened 7 years ago Closed 2 years ago

severe jank when deleting sites from the history sidebar

Categories

(Toolkit :: Places, defect, P3)

defect

Tracking

()

RESOLVED DUPLICATE of bug 734643
Performance Impact low

People

(Reporter: froydnj, Unassigned)

References

Details

(Keywords: perf)

I witnessed: https://perfht.ml/2qdYXTu today. Go to "Stack Chart" and you'll see very expensive stacks happening beneath `nsNavHistory::RemovePagesFromHost`. Notice also the multi-second event processing delay in the parent process. STR to reproduce are approximately: 1. Visit several pages from the same host (10-20 seems sufficient for me). 2. Open History sidebar. 3. Switch to "By Date and Site" view. 4. Click on the site visited in step #1 to select it. 5. Hit the "Delete" key. 6. Watch your browser hang. The more pages you visit, the longer your browser hangs. This is with a ~two week old Nightly, though, and there might have been some performance improvements in this area recently? Doug, does this sound like something you can look into?
Flags: needinfo?(dothayer)
(In reply to :froydnj (on leave until 2018, email/ni? if necessary) from comment #0) > today. Go to "Stack Chart" and you'll see very expensive stacks happening > beneath `nsNavHistory::RemovePagesFromHost`. Notice also the multi-second > event processing delay in the parent process. Bug 1089691. That API is old and synchronous, we have a new API but we didn't convert the call points yet.
Depends on: 1089691
Priority: -- → P3
(I've looked at this a bit but I'm leaving the needinfo for now. This is a bit outside the scope of migration, which is why I'm currently in this code, but I wouldn't mind looking into fixing this if it's not too much of a lift.)
Flags: needinfo?(dothayer)
There are various reasons for this and thus it's not trivial, but it's definitely fixable: 1. as stated in comment 1, still uses the old sync API, conversion is in progress 2. Places notifications in general are horribly designed (bug 1340498) 3. We are in the process of removing a bunch of old APIs and we didn't have time to look into views optimizations yet (also to be noted that all the views are doing some synchronous I/O because the whole MVC system should be rewritten and that requires UX to suggest a new UI approach for things like bookmarks and history)
(In reply to Nathan Froyd [:froydnj] from comment #0) > today. Go to "Stack Chart" and you'll see very expensive stacks happening > beneath `nsNavHistory::RemovePagesFromHost`. Notice also the multi-second > event processing delay in the parent process. > Note that in at least part of that reported multi-second event processing delay, we're spinning the event loop inside some Sync code (presumably reacting to changes to History), so the event processing delay marker is a bit misleading here (we _are_ servicing events, just inside a nested loop).
Whiteboard: [qf] → [qf:64][qf:p3]
Whiteboard: [qf:64][qf:p3] → [qf:f64][qf:p3]
Whiteboard: [qf:f64][qf:p3] → [qf:p3:f64]
Whiteboard: [qf:p3:f64] → [qf:p3]
Performance Impact: --- → P3
Whiteboard: [qf:p3]
Severity: normal → S3
Status: NEW → RESOLVED
Closed: 2 years ago
Duplicate of bug: 734643
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.