836488 - Use read-ahead when loading plugin shared libraries

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect)

Description

[Vladan Djeric (:vladan)]

I frequently get main-thread jank of 100ms or more from the initial load of the Flash library. This is also the #1 source of chrome-hangs:

PR_WaitCondVar (in nspr4.dll)
 -> mozilla::CondVar::Wait(unsigned int) (in xul.dll)
 -> mozilla::ipc::GeckoChildProcessHost::SyncLaunch (in xul.dll)
 -> mozilla::plugins::PluginProcessParent::Launch(int) (in xul.dll)
 -> mozilla::plugins::PluginModuleParent::LoadModule(char const *) (in xul.dll)
 -> GetNewPluginLibrary(nsPluginTag *) (in xul.dll)
 -> nsNPAPIPlugin::CreatePlugin(nsPluginTag *,nsNPAPIPlugin * *) (in xul.dll)
 -> CreateNPAPIPlugin (in xul.dll)
 -> nsPluginHost::EnsurePluginLoaded(nsPluginTag *) (in xul.dll)

We should figure out if reading plugin libraries with the read-ahead flag (à la xul.dll) will help with plugin jank.  We can start by adding a histogram similar to PLUGIN_SHUTDOWN_MS.

* Preloading the Flash library after startup is another possibility but we can look into that in a separate bug if this fails.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Adds telemetry for plugin library load timing

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 710811 [details] [diff] [review]
Adds telemetry for plugin library load timing

Cancelling review request, this needs more work

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Adds telemetry for plugin library load timing rev. 2

Here's the corrected version: Added telemetry for launching plugin-container in addition to in-process plugins. The histograms are separate for in-process vs OOP so that we aren't comparing apples to oranges.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Adds telemetry for plugin library load timing rev. 3

Forgot to include Telemetry.h

Comment 5

[Benjamin Smedberg]

Comment on attachment 710867 [details] [diff] [review]
Adds telemetry for plugin library load timing rev. 3

The probe you have here won't actually hit the jank stack from comment 0: this codepath is for in-process plugins only (so only Java on Windows by default).

But note that probably most of the time spent here is in launching the process, not loading the library. We do also sometimes load the library (on Windows) in-process in order to pull the metadata such as the plugin MIME types/versions which are stored in the resource block.

Comment 6

[Vladan Djeric (:vladan)]

The load times might be less variable if we limit ourselves to only timing the Flash library

Comment 7

[Vladan Djeric (:vladan)]

^ as opposed to other plugins

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> Comment on attachment 710867 [details] [diff] [review]
> Adds telemetry for plugin library load timing rev. 3
> 
> But note that probably most of the time spent here is in launching the
> process, not loading the library. We do also sometimes load the library (on
> Windows) in-process in order to pull the metadata such as the plugin MIME
> types/versions which are stored in the resource block.
My idea was that measuring the time from plugin-container launch to opening of the channel could be a proxy for the load time. We're not really interested in the load time itself; we're more interested in the change in load times between now and after the readahead patch lands.

Any suggestions?

Comment 9

[(dormant account)]

Benjamin is there a better way to measure plugin dll-loading overhead for out-of-process plugins? We need to test whether readahead will reduce occurrences of blocking on stack in comment 0.

Comment 10

[Benjamin Smedberg]

I think this got resolved in person: measuring process launch time is going to be really noisy, so it would be valuable to measure the in-process load time (just for metadata) and to measure the load times in the other process I think we need to measure directly there.

Comment 11

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Adds telemetry for plugin library metadata load timing

This patch is to measure the plugin metadata load time as discussed earlier this week.

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch for adding plugin readahead

This is the patch that actually implements the plugin readahead. This won't land until after we have gathered sufficient telemetry for the "before" case.

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

Telemetry patch:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b23e5f546045

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b23e5f546045

Comment 15

[Vladan Djeric (:vladan)]

Are we landing the 2nd patch?

Comment 16

[(No longer employed by Mozilla) Aaron Klotz]

Data gathered via SystemTap runs showed a significant increase in load time when testing with libflashplayer.so with readahead.

Average load time without readahead: 5419ms
Average load time with readahead: 13551ms

(Note that the hardware used for this test is fairly old, so the non-readahead baseline is pretty bad to begin with)

Comment 17

[Georg Fritzsche [:gfritzsche]]

So was this found to not improve the situation and can be closed?

Comment 18

[(No longer employed by Mozilla) Aaron Klotz]

Correct.