Open Bug 1701660 Opened 7 months ago Updated 1 month ago

Performance issues while importing a large CSV file with ~1000 logins

Categories

(Firefox :: about:logins, defect, P2)

Desktop
All
defect

Tracking

()

ASSIGNED
Tracking Status
firefox87 --- disabled
firefox88 --- disabled
firefox89 --- affected

People

(Reporter: cmuntean, Assigned: sgalich, Mentored)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files)

[Notes]:

  • Importing a CSV file with 1000 logins takes between 3 to 10 minutes. During the import, there are multiple performance issues that can be observed. The performance issues get worse after ~800/1000 logins are imported. Here are a few examples of performance issues observed:

    • Firefox can’t navigate to any page using the address bar until all logins have been imported.
    • The top sites from the new tab, cannot be opened while the CSV file is imported. If one of the top sites is clicked, then a new tab is opened, the page remains blank until the logins are imported.
    • In certain cases, Firefox also freezes. This happens after a few minutes while the importing process is about to end (when ~850 logins are imported).
    • If the about:logins page is closed while importing a CSV file then if it's reopened, the page remains blank until all the logins are imported..
    • The hover effect has a delay.
    • Opening new tabs or Firefox menus have a delay.
  • Considering the fact that this is a performance issue we have captured a performance profile using the Firefox Profiler addon on multiple machines:

[Affected versions]:

  • Firefox Nightly 89.0a1 (Build ID:20210328213901);
  • Firefox Beta 88.0b (Build ID: 20210328185936)

[Affected Platforms]:

  • Windows 10 x64;
  • macOS 10.15.7;
  • Linux Mint 20 x64;

[Prerequisites]:

  • Have the latest Firefox Beta build installed.
  • Have a large CSV file that contains multiple logins or you can use this one (1004 logins): file.

[Steps to reproduce]:

  1. Open the Firefox browser with the profile from prerequisites.
  2. Navigate to the "about:logins" page.
  3. Open the menu, click the "Import from a file..." option.
  4. Select and import the CSV file described in perquisites.
  5. Wait until ~800 logins are imported.
  6. Open a new tab, click on the top sites, navigate to multiple websites.
  7. Open different Firefox menus and click on options.
  8. Observe how the browser behaves.

[Expected result]:
There are no noticeable performance issues.

[Actual result]:

  • Different performance issues can be observed.

[Additional Notes]:

  • Attached a screen recording of the issue.
See Also: → 1702336

A quick glance shows some low hanging fruit here. We shouldn't _setListItemAsSelected and scrollIntoView here when we are importing. We might have to refactor things though to avoid this, as we simply react to the loginAdded message in the same way as if the user had just created a single new login entry.

https://searchfox.org/mozilla-central/source/browser/components/aboutlogins/content/components/login-list.js#719-734

We may need to signal from the importer that we are expecting a batch of new records and signal again when the import is done so the UI can flush and re-draw once instead of per new record.

This feature is held to nightly only for now, so I'll mark as P2. But it should be the first thing we address to try and get the feature ready for shipping.

Priority: -- → P2
Blocks: 1705529
Assignee: nobody → tgiles

Adding a link to my investigation of this bug. There a lot of synchronous operations we are doing on the main thread in multiple places during the import process. We need to solve those as well as optimize the rendering pipeline for login items.

I don't believe solving these performance issues will result in particularly novel solutions. Because of this, we should have some resources on file on how to mitigate and solve these kind of performance issues. If we don't have these resources, then this bug could be a good case study of how to use the profiler, moving main thread IO off the main thread (or minimizing main thread IO if we can't move all the work off the main thread), and optimizing the pixels to screen pipeline. The performance best practices for Firefox front-end engineers highlights these issues at a high level so maybe this bug can expand on those high level points.

Assignee: tgiles → nobody
See Also: → 1658766

This is not a trivial bug, but if someone wants to investigate and develop a patch, I'm very happy to mentor them through it. I'm happy to add to Tim's notes (comment #4) and chat to help guide a contributor through a step by step process to solving this. We don't need to boil the ocean here, we should start with profiling, identifying the main sources of slowness, setting up to respond to batch rather than one-by-one login additions and take it from there.

Mentor: sfoster
Assignee: nobody → sgalich
Status: NEW → ASSIGNED
You need to log in before you can comment on or make changes to this bug.