1391590 - [meta] The first PlacesUtils.bookmarks.fetch call does main thread IO to initialize the places db connection

Status: NEW

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

Description

[Florian Quèze [:florian]]

See this startup profile where the main thread is blocked for more than 200ms while initializing the places database connection to update the star icon: https://perfht.ml/2v7kYQY

This jank is caused by this PlacesUtils.bookmarks.fetch call http://searchfox.org/mozilla-central/rev/e8c36327cd8c9432c69e5e1383156a74330f11f2/browser/base/content/browser-places.js#1710 that looks like its async but actually blocks.

Comment 1

[Marco Bonardo [:mak]]

As I previously said, Places database init is still synchronous and on main-thread. Changing that requires a lot of work and no more synchronous APIs around, otherwise we couldn't wait for the connection to be ready. We're moving towards that goal, but we're still not there.

The best we could do so far is bug 1371677 and until we are at a point where all the synchronous APIs are gone, the only thing we can do is further delay consumers.
The first consumer actually doing a read or a write, will pay the connection opening price.
In this case the star button wants to read an info from the db, and there no way out unless we either delay the star button init, or skip completely the lookup for well known startup pages (about:home, about:newTab), but then the star state may be wrong.

Note that this profile is measuring the worst case, that is a new profile, the database doesn't exist and the Windows CreateFileW API is invoked. Reopening an existing db should be cheaper than creating a new one.
Now that we also have favicons.sqlite, we spend time on a second CreateFileW API call to create that file too.

These 2 CreateFileW API calls are covering most of the profile time.

Comment 2

[Florian Quèze [:florian]]

(In reply to Marco Bonardo [::mak] from comment #1)

> Note that this profile is measuring the worst case, that is a new profile,
> the database doesn't exist and the Windows CreateFileW API is invoked.

It was an existing profile. CreateFile is a very misleadingly named Windows API. I wish it had been called "CreateFileHandle". The first sentence of its msdn documentation says "Creates or opens a file or I/O device." https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx

Comment 3

[Marco Bonardo [:mak]]

Thanks for the clarification. I therefore assume this is a best case (close to average since the worst case happens only once).
Unfortunately it doesn't change things we can and cannot do, but it's good to have a hint of how much time we take for setting up the connection. Long term the sync->async project should allow us to just open the connection async.

Comment 4

[Bas Schouten (:bas.schouten)]

Places related sqllite stuff seems to be responsible for ~1.4% of the current BHR reports. Is there any path forward here, or anyone working at these issues?

Comment 5

[Marco Bonardo [:mak]]

The team working on bookmark improvements it also looking into possibly initializing the bookmarks toolbar in an async way, but converting the connection to async requires a lot more resources we don't have to complete the transition to async (convert the tagging API and rewrite all the UI views)