Closed
Bug 882339
Opened 11 years ago
Closed 11 years ago
Ts and Tp regression from bug 875454/bug 880675
Categories
(Core Graveyard :: Plug-ins, defect, P1)
Tracking
(firefox23 unaffected, firefox24+ fixed)
RESOLVED
FIXED
mozilla24
Tracking | Status | |
---|---|---|
firefox23 | --- | unaffected |
firefox24 | + | fixed |
People
(Reporter: benjamin, Assigned: benjamin)
References
Details
(Keywords: perf, regression)
Attachments
(1 file)
There were Ts/Tp regressions from bug 875454/bug880675. I believe that the problem is that calling into the blocklist service is rather expensive, and we do it a fair bit when walking through enabled plugin tags and whatnot.
My first plan is to cache the blocklist state on the plugin tag, but not persist it anywhere. This should completely fix Tp. I'm a little worried about Ts because we may be now loading the blocklist service prior to first-paint and if so I'm not sure what we can do about that. But first patch first.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #762119 -
Flags: review?(jschoenick)
Assignee | ||
Comment 2•11 years ago
|
||
Oops the change in test_bug391728.html is a leftover, it shouldn't be necessary.
Updated•11 years ago
|
Assignee | ||
Comment 3•11 years ago
|
||
passes tryserver: https://tbpl.mozilla.org/?tree=Try&rev=c7c491b67c3f
Updated•11 years ago
|
Attachment #762119 -
Flags: review?(jschoenick) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee | ||
Comment 5•11 years ago
|
||
But it didn't work. Talos did not budge at all with this patch.
I'm a little worried now:
* Perhaps this caching code is not being effective because blocklist->GetPluginBlocklistState is throwing an exception in the talos environment? I'd like to understand whether the blocklist is enabled or disabled in Talos and what other non-default configs it uses
* Perhaps the blocklist code is not the problem, and it's actually the permission manager which is causing the slowdown. I'll know more with some profiling on today's nightly. But given that information, we'd basically have to fix the permission manager, since using the permission manager is fundamentally necessary to making plugins click-to-play by default.
Is it possible to figure out if specific Tp pages are responsible for this slowdown, or whether it's spread evenly across a lot of pages?
Status: RESOLVED → REOPENED
Flags: needinfo?(jmaher)
Resolution: FIXED → ---
Comment 6•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 7•11 years ago
|
||
I looked at this with bsmedberg via irc. The extension blocklist is set to 127.0.0.1/dummy/..., basically a 404. We added this about a month ago to talos as we found there was network access requests getting sent out. We don't need the entire list of dummy server, but we did need the extensions.update.background.url to prevent nss crashes on android:
https://bugzilla.mozilla.org/show_bug.cgi?id=871575#c14
I have looked at a before/after of the test run in more details (per page, not single value for the entire run) and compiled a spreadsheet:
https://docs.google.com/spreadsheet/ccc?key=0ArS97F99-BEZdEgzVkd3ZE1aUFc0WjVONno2ZzNiOVE#gid=0
There are some large increases, worth looking into.
Flags: needinfo?(jmaher)
Assignee | ||
Comment 8•11 years ago
|
||
I'm now very confused about this. The highest regressions appear to be in imgur.com and reddit.com. I loaded those pages in a local browser and I see:
* Neither of these pages loads any plugins
* when loading these pages, I don't trip breakpoints on any of the following functions:
** nsObjectLoadingContent::ShouldPlay
** nsObjectLoadingContent::LoadObject
** nsContentDLF::CreateInstance at the point where we check for plugin MIME types
** nsPluginTag::GetBlocklistState
** nsPluginHost::PluginExistsForType
The two regression changesets are:
https://hg.mozilla.org/mozilla-central/rev/5ef49706a56a
https://hg.mozilla.org/mozilla-central/rev/ea3191239e8e
It appears that those functions are the only entry points for changes in these bugs, which leaves me quite stumped as to what is different. johns, do you have any other suggestions before I back out one or the other of these patches?
Flags: needinfo?(jschoenick)
Assignee | ||
Comment 9•11 years ago
|
||
johns helped me figure it out! These pages are enumerating navigator.plugins (more than once in some cases), *and* there was a bug in my caching where nsPluginTag::IsBlocklisted wasn't using the cache.
Got r=johns over IRC for the obvious fix. I now owe lots of people excellent libations.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4fb8fa8668c5
Flags: needinfo?(jschoenick)
Comment 10•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•