1387181 - DB should not be accessed on main thread

Status: RESOLVED DUPLICATE of bug 1404087

(Firefox for iOS :: General, defect, P3)

Description

[:garvan]

For a responsive UI, this should never happen.
Here is the app starting from scratch, and some of the stack traces where DB access is happening on the main thread.

The code I used to grab samples of the stack trace: https://pastebin.mozilla.org/9028780

Stacktraces:

From cold start: https://pastebin.mozilla.org/9028781
Tapping history panel: https://pastebin.mozilla.org/9028783

Comment 1

[:garvan]

Richard: pinging so you are aware of this (just clear when read). We might want to plan to review all these cases and ensure they are by-design.

I am not sure if we have load tests for large DB datasets, particularly to verify the UI is not blocked.
Anecdotally, switching home panel views on my iPhone 5C blocks the UI, I would guess I have an average-sized Sync profile.

Comment 2

[Richard Newman [:rnewman]]

(In reply to :garvan from comment #1)

> Anecdotally, switching home panel views on my iPhone 5C blocks the UI, I
> would guess I have an average-sized Sync profile.

All of our DB operations block the DB thread; that's expected.

Additionally, writes -- by default -- block the calling thread. In part that's because it's a callback-oriented API.

Any object initialization that does a write -- including establishing BrowserTable, RemoteClientsTable, and opening the profile -- will block the initializing thread. Often that will be the main thread.

Bug 1388147 will help with this by eliminating a synchronous exclusive transaction the first time a table is accessed, and a synchronous exclusive transaction during startup. Gradually eliminating ORMish code left in the codebase will help further.

There aren't actually that many places in the UI that write to the DB, fortunately. Sync uses its own queue.

Some of those UI places use `BrowserDB.run`, and already specify `synchronous: false` (e.g., bookmarking). Others (e.g., `SQLiteLogins.addUseOfLoginByGUID`) don't. Others go lower-level, using `db.withConnection` directly, and mostly do so synchronously (e.g., `addLocalVisitForExistingSite).

Proper symbolicated stacks pulled from Xcode would be handy, Garvan! Those pastebins are kinda hard to read :)

Comment 3

[:garvan]

I think we agree writes blocking the main thread is a performance problem, we could start by debug-asserting in the DB code if this is happening. Probably we would have to take a day, add that in, and have everyone available to fix these usages. Any future bad actors would get caught then.

Profiling may not reveal much, as writes are often fast, except when they are not :). Best just not to write on main thread.
Disk I/O contention for writes on iOS is part of the problem here, as writes that routinely take 50ms can spike to multiples of that (although writes are slow enough as-is). 
The boilerplate core data stack (and the new iOS 10 NSPersistentStoreCoordinator which encapsulates the recommended stack, plus moves all DB init code off-main) ensures writes are not possible on the main thread.

I would recommend as part of our application design we would specify no code should read or write on the main thread to ensure the UI is as snappy as possible.

(I see ETLD file being read on main thread, perhaps our tab archiving code is on main, perhaps various app startup code is doing I/O on main thread and should be off-main or deferred as late as possible.)

Comment 4

[Justin D'Arcangelo [:justindarc]]

We fixed this in Bug 1404087. Closing.