1363425 - _migrateFromRDF causes main thread IO during startup

Status: VERIFIED WORKSFORME

(Firefox :: File Handling, defect, P2)

Description

[Florian Quèze [:florian]]

See this profile: https://perf-html.io/public/5727bef72dac23c0565f9d20615df6e6251a9200/calltree/?callTreeFilters=prefix-01GkzXkzXlzYazYbzYczYdBJjCy5FW8FW2Cy8Cy9EW6CybCycCydxJ9CyeCyfALuADrGEcGDkCygADvAE0FYfGDbAEcGH6EVeG9xReDUh_nySpEVeG9CyhDUh_nySpEVexHgyQtEIhRaRbImInIoBziIqySpEVeG9zQuCImDUh_nySpEVexLkI9ySpEVeG9CIpCJ1DUh_nCwhFF9GH6EVeG9CJ2CJ3DUh_nySpEVexLkI9ySpEVeG9CJ4DUh_nySpEVexHgyQtEIhCJ5ImInIoBziIqySpEVeG9CJrDUh_nySpEVeG9CJsDUh_nCwhFF9GH6EVeG9CJtDUh_nySpEVeG9CJuDUh_nySpEVeG9CK0&range=0.0000_8.7709&thread=0

Most of the time is spent in nsOSHelperAppService::GetApplicationDescription(nsACString const &,nsAString &) which calls the Windows AssocQueryStringW API that does main thread IO.

Comment 1

[:Paolo Amadini]

We have bugs on file to remove all unneeded OS accesses as much as possible, but what is triggering the helper app service during startup in the first place?

Comment 2

[Florian Quèze [:florian]]

(In reply to :Paolo Amadini from comment #1)

> what is triggering the helper app service during startup in the first place?

http://searchfox.org/mozilla-central/rev/7057a51c5cf29b5b115b1db19ace2cfe3fd83a0e/browser/extensions/pdfjs/content/PdfjsChromeUtils.jsm#302
triggered by:
http://searchfox.org/mozilla-central/rev/224cc663d54085994a4871ef464b7662e0721e83/browser/components/nsBrowserGlue.js#882

See https://perf-html.io/public/5727bef72dac23c0565f9d20615df6e6251a9200/calltree/?callTreeFilters=prefixjs-xReCyhzQuCIm&implementation=js&range=0.0000_8.7709~3.7932_5.7545&thread=0

Comment 3

[:Paolo Amadini]

It seems that fixing bug 1362401 will solve this.

Comment 4

[Florian Quèze [:florian]]

Will it fix it, or just cause it to happen after startup (potentially when the user is trying to interact with the browser)?

Comment 5

[:Paolo Amadini]

(In reply to Florian Quèze [:florian] [:flo] from comment #4)
> Will it fix it, or just cause it to happen after startup (potentially when
> the user is trying to interact with the browser)?

Just cause it to happen after startup. While we'll incur in the full migration cost once, main-thread I/O for "handlers.json" will be still required when nsIHandlerService is used. This is very difficult to fix because the content type handling determination is synchronous.

Comment 6

[Andrew Overholt [:overholt]]

Is this only going to happen once during migration?

Comment 7

[:Paolo Amadini]

(In reply to Andrew Overholt [:overholt] from comment #6)
> Is this only going to happen once during migration?

The major issues happen only once during migration when we read all the file types, but until bug 1362401 is fixed we may still have a little impact based on how the PDF file type is configured in the operating system.

Comment 8

[:Gijs (he/him)]

(In reply to :Paolo Amadini from comment #5)
> While we'll incur in the full
> migration cost once, main-thread I/O for "handlers.json" will be still
> required when nsIHandlerService is used

Can we use OMT IO to initialize it on idle well after startup, avoiding main thread IO in the majority of cases?

Comment 9

[:Paolo Amadini]

It's feasible, in fact this is what we do for "logins.json". However, given that we still import data from "mimeTypes.rdf" when necessary, there may be race conditions to be aware of, so it may be non-trivial.

This bug was originally filed for attempts to read the file type associations during startup. If these attempts are now fixed, I'm not sure of what is the impact of the main-thread I/O here. This will probably affect the first page load of a content type that we cannot handle internally.

Comment 10

[Marco Bonardo [:mak]]

(In reply to :Paolo Amadini from comment #9)
> It's feasible, in fact this is what we do for "logins.json". However, given
> that we still import data from "mimeTypes.rdf" when necessary, there may be
> race conditions to be aware of, so it may be non-trivial.

fwiw, mimetypes.rdf migration is gone now.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

(In reply to Marco Bonardo [::mak] from comment #10)
> (In reply to :Paolo Amadini from comment #9)
> > It's feasible, in fact this is what we do for "logins.json". However, given
> > that we still import data from "mimeTypes.rdf" when necessary, there may be
> > race conditions to be aware of, so it may be non-trivial.
> 
> fwiw, mimetypes.rdf migration is gone now.

\o/