Closed Bug 1795070 Opened 2 years ago Closed 2 years ago

Deleting from history search is extremely slow

Categories

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

Firefox 105
defect

Tracking

()

VERIFIED FIXED
108 Branch
Tracking Status
firefox108 --- verified
firefox109 --- verified

People

(Reporter: mckenfra, Assigned: mckenfra)

References

Details

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:105.0) Gecko/20100101 Firefox/105.0

Steps to reproduce:

  1. Open browsing history.
  2. Search for a keyword that brings back 2,000 history items.
  3. Select them all and press delete.

(Use attached file to artificially create a large browsing history.)

Actual results:

It took 73 seconds to delete 2,000 history items.

During this time, the number of history items appeared to slowly decrease in batches of 300, with long pauses inbetween each batch.

Expected results:

It should have taken about 1 second to delete 2,000 history items.

Initial investigation revealed that when the user deletes 2,000 items from a history search, treeView.js invalidateContainer() is invoked 2,000 times (in batches of 300 at a time). However, this is not the primary cause of the poor performance.

Yes, one of the nsNavHistoryQueryResultNode observers is being refreshed for every Page_removed event, resulting in repeated calls to treeView.js invalidateContainer(). This definitely has a performance impact.

However, even worse is that another of the nsNavHistoryQueryResultNode observers is being incrementally updated for every Page_removed event. This is causing the majority of the slowdown.

This patch provides a simple solution: when handling a PlacesEvent containing 50 or more Page_removed events and no other events, just refresh all the history observers once and return.

With this change, it takes about 1 second to delete 2,000 items from a history query, representing a performance improvement of approximately 7,000%.

Assignee: nobody → 3ecdbelo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

The Bugbug bot thinks this bug should belong to the 'Firefox::Bookmarks & History' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Bookmarks & History
Blocks: 734643
Severity: -- → S3
Priority: -- → P2
Priority: P2 → P3
  1. In _removeRowsFromHistory(), wrap remove() in PlacesUIUtils.batchUpdatesForNode()
  2. In _removeHistoryContainer(), close the container before proceeding
  3. In HandlePlacesEvent(), use optimised "bulk Remove_page event"-handling in more situations

Depends on D159286

Attachment #9301550 - Attachment is obsolete: true
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/04e6abff2792
Speed up deleting from history by 7,000%. r=mak
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch
Flags: qe-verify+

Reproducible on a 2022-10-13 Nightly build on macOS 12 using the STR and testcase from Comment 0.
Verified as fixed on Firefox 108.0b3(build ID: 20221117185908) and Nightly 109.0a1(build ID: 20221117214129) on macOS 12, Ubuntu 22, Windows 10.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: