1795070 - Deleting from history search is extremely slow

Status: VERIFIED FIXED

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

Description

[Francis :mckenfra]

Attachment: Artificially generate a large number of history items. Buttons to start/stop.

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.

Comment 1

[Francis :mckenfra]

Attachment: Bug 1795070 - Speed up deleting from history by 7,000%. r=mak,Standard8

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%.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 3

[Francis :mckenfra]

Attachment: Bug 1795070 - Reviewer-suggested changes. r=mak

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/04e6abff2792
Speed up deleting from history by 7,000%. r=mak

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/04e6abff2792

Comment 6

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

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.