987745 - Avoid main-thread IO for formhistory.sqlite

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Roberto Agostino Vitillo (:rvitillo)]

It seems that formhistory.sqlite, formhistory.sqlite-wal and formhistory.sqlite-journal are top main-thread IO offenders in terms of number of Telemetry submissions where they appear in.

Comment 1

[Marco Bonardo [:mak]]

this component uses async APIs for most stuff but initialization, so this is a little bit surprising.
how is the measure done? does it include first startups of the browser?

Comment 2

[Marco Bonardo [:mak]]

Mostly, if this is one of the top offenders, I'm worried about our measurements.

Comment 3

[Roberto Agostino Vitillo (:rvitillo)]

Startup and shutdown IO operations have been filtered out so only IO operation during execution are counted (see bug 976000). 

Aaron, is there any way an IO operation performed during startup could be miscounted as performed after initialization?

I see a median number of IO operations of about 3, which is not a lot. But considering that about 50% of Nightly's population performs main-thread IO during execution on formhistory.sqlite and that a even a single and simple operation might in some circumstances take a while to complete, it still worth looking into it.

Comment 4

[Marco Bonardo [:mak]]

(In reply to Roberto Agostino Vitillo (:rvitillo) from comment #3)
> I see a median number of IO operations of about 3, which is not a lot. But
> considering that about 50% of Nightly's population performs main-thread IO
> during execution on formhistory.sqlite and that a even a single and simple
> operation might in some circumstances take a while to complete, it still
> worth looking into it.

I mostly wonder what do we mean by "in terms of number of Telemetry submissions where they appear in".. clearly database connections must be open every time a profile is opened, those are still on main-thread for most of our databases (it's possible to convert them to not act on main-thread, but requires non trivial amount of work). So yes, I expect any profile in the world to report main-thread IO opening the database, but that's all of the IO I expect, so when prioritizing things this doesn't look like a top offender, but just an offender. Unless there's something obscure we don't know about our IO patterns.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Roberto Agostino Vitillo (:rvitillo) from comment #3)
> Startup and shutdown IO operations have been filtered out so only IO
> operation during execution are counted (see bug 976000). 
> 
> Aaron, is there any way an IO operation performed during startup could be
> miscounted as performed after initialization?

For both xperf and main thread I/O telemetry, sessionstore-windows-restored is considered to be the cutoff for treating I/O as startup I/O, so it depends on which side of that event the I/O falls upon.

Comment 6

[Marco Bonardo [:mak]]

ok, so, take my comment as "I think just counting global number of calls is not a very good way to find top offending stuff". It's true that even just a single call is very bad, but there are definitely worse offenders that do all of their IO on main-thread. Just from a prioritization point of view, clearly. This should be fixed regardless.

Comment 7

[Roberto Agostino Vitillo (:rvitillo)]

(In reply to Marco Bonardo [:mak] from comment #6)
> ok, so, take my comment as "I think just counting global number of calls is
> not a very good way to find top offending stuff". It's true that even just a
> single call is very bad, but there are definitely worse offenders that do
> all of their IO on main-thread. Just from a prioritization point of view,
> clearly. This should be fixed regardless.

Agreed, top offender is a bad choice of wording. The reality is that we mostly got rid of all the heavy offenders and the few still out there have been already reported. Relatively speaking, with what remains left to be moved off the main-thread (and reported), formhistory.sqlite is fairly highly ranked.

Also, the ranking is done based on the frequency that a file appears in a submission. Imho removing a single call for a file that appears in 50% of the population is a bigger win than removing 50 calls from a file that appears in only 1% of the population. This is just one of the ways in which we rank those submissions.

Comment 8

[Marco Bonardo [:mak]]

Sounds good, thanks for the clarifications.

Comment 9

[Vladan Djeric (:vladan)]

See also bug 887887 and bug 876002

Comment 10

[Marco Bonardo [:mak]]

It should now be completely asynchronous thanks to bug 888784.