investigate new NPAPI plugin unloading strategy

RESOLVED FIXED in mozilla13

Status

()

Core
Plug-ins
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

Trunk
mozilla13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

8 years ago
Bug 500925 changed our default behavior to never unloading NPAPI plugins once they are loaded. We should investigate a new strategy for unloading plugins that does not regress the following scenario:

"1 window open, 1 tab: Open page X which has a single Flash instance. Go to page Y, which also has a single Flash instance."

In this scenario, if we unload plugins when their last instance is destroyed then we'll completely unload the Flash plugin when we unload page X and load the whole plugin again when we load page Y. We don't want to do that, we want to simply destroy the first instance and then load another instance, keeping the plugin alive the whole time.

Note that the bfcache does not keep plugin instances alive.

A potential solution is to see whether or not a plugin can be unloaded some number of minutes after its last instance is destroyed, killing that timer if another instance is started.
There's also a memory pressure notification that stuart added, which would be a good time to free any unused plugins.

Comment 2

8 years ago
Firefox 3.6 is never unloading the plugins anymore. While this is a very nice speech improvement for websites like Youtube and such, wouldn't people start reporting this as a memory leak (the behavior change is not publicly known yet)? Especially Flash can consume quite a lot of memory.
Duplicate of this bug: 535323

Comment 4

7 years ago
In the duplicate bug that I filed (see comment 3), I stated the following:

Results after watching a video:

Just opening youtube.com was around 6,000 K
Closing youtube.com without watching a video resulted in releasing 100 K
Watching the video memory went to 39,000 K
closing the youtube tab and memory went down to 13,000 K

This is going to appear as a memory leak to any end user.  So users that complain of memory being held by plugins that might be used
again in same session should add 'plugins.unloadASAP'?  I just foresee a lot of
complaints from users complaining about the memory being held.

I also didn't get an answer about this, "I couldn't find in the bug you linked to the actual time limit until the
process is killed.  There was talk of 10 minutes but that doesn't seem like it
was the final answer."  Bug 500925 is the mentioned bug
Duplicate of this bug: 650948
Duplicate of this bug: 656326
Duplicate of this bug: 657753
Duplicate of this bug: 538826

Updated

6 years ago
Duplicate of this bug: 662028

Comment 10

6 years ago
(In reply to comment #0)
> A potential solution is to see whether or not a plugin can be unloaded some
> number of minutes after its last instance is destroyed, killing that timer
> if another instance is started.

Here is a solution, 
1.we differ the setting of each plug-in, and have a gerneral killing time keep on record the time since its last used, says the threshold 2mins.

2. when first time a plug-in meets the time, pop-up asking user "whether auto kill the instance when it's unused next time?" as we ask them whether keep a password.

So user can kill the less using plug-ins and keep the active ones (like Flash)
OS: Mac OS X → All
Hardware: x86 → All

Updated

6 years ago
Duplicate of this bug: 673356

Comment 12

6 years ago
The processes after closed takes a long time to release memory.

Comment 13

6 years ago
Firefox should terminate the process when you disable the plugin in the AOM.

Comment 14

6 years ago
I mean to say that if I close any flash website, then the plugin-container still consumes memory and is not released immediately.

I don't want to disable the plugin.

Comment 15

6 years ago
I wasn't responding to you.

This bug has nothing to do with flash or flash plugin container memory usage.

Comment 16

6 years ago
Then why my bug Bug 673356 has been marked as a duplicate of this bug.
Comment 15 is just wrong, as far as I can tell. This bug is precisely about deciding when to kill plugin-container when there are no plugin instances around.

Note that "immediately" would actually suck a lot on page transitions, so is probably the wrong answer.  Hence the "investigate" bit.

Comment 18

6 years ago
Its not about the container taking a long time to release memory, thats a flash bug itself.

Updated

6 years ago
Blocks: 675471
Whiteboard: [MemShrink]
Duplicate of this bug: 675471
Whiteboard: [MemShrink] → [MemShrink:P2]
Blocks: 563227

Updated

6 years ago
Duplicate of this bug: 688530

Comment 21

6 years ago
The workaround I am using is to close firefox when I see too many plugin-container.exe processes running, and open it again. This cannot be, naturally,
the final solution. The browser cannot start new processes and never terminate
them when they are no longer needed. So, if it is really time consuming to
start them when need occurs, let's keep them alive for some time when they are no longer needed (in case the user would open shortly another page requiring them).
Duplicate of this bug: 688496
Duplicate of this bug: 693820

Updated

6 years ago
Duplicate of this bug: 701951
I'm going to take this, but I'll want plugin-as-content to land first otherwise the merge will be difficult.
Assignee: nobody → benjamin

Comment 26

6 years ago
i can't seem to find a bug for that, is there one?
bug 90268

Updated

5 years ago
Duplicate of this bug: 711825
Depends on: 90268

Updated

5 years ago
Duplicate of this bug: 720235

Comment 30

5 years ago
I have opened a ticket too but it was marked as a duplicate (I read the first post of this ticket before but it hasn't described the closing of plugin-container (was there already plugin-container at this date?)). I would like to have something like dom.ipc.plugins.idleTimeoutSecs which defines the time in seconds after plugin-container will be closed if no plugin used it.

Comment 31

5 years ago
the plugin container loads the plugin, not the other way around ;p

for npapi unloading to work, content and the container need to notify each other of whats going on is the gist i get from this, or else you could terminate things like buffered video's simply because theres no activity after the video buffers completely.

Comment 32

5 years ago
>the plugin container loads the plugin, not the other way around ;p

I know this but my post seems to look a little confusing about this :P

>for npapi unloading to work, content and the container need to notify each other of whats going on is the gist i get from this, or else you could terminate things like buffered video's simply because theres no activity after the video buffers completely.

Normally after a video is buffered it is playing and the flash plugin is processing it. Even if I pause a video there is a moderate cpu usage. If I kill the plugin-container the video says that the flash plugin has crashed.

Doesn't Firefox already know with this way that the flash plugin is already working? Otherwise how does Firefox know that the flash plugin has crashed? Just by watching if plugin-container is running?
(Assignee)

Comment 33

5 years ago
Benjamin - do you mind if I take this bug or are you planning to work on it soon?
(Assignee)

Comment 34

5 years ago
Created attachment 596059 [details] [diff] [review]
delayed unload strategy, v1.0

I wrote this pretty quickly but it seems to work alright. I'll need to think about this and do some more testing to be confident, but it'd be good to get review and feedback now.
Attachment #596059 - Flags: review?(benjamin)
(Assignee)

Comment 35

5 years ago
Created attachment 596140 [details] [diff] [review]
delayed unload strategy, v1.1

Fixes a minor bug that probably had no real consequences (incorrect return value from timer Notify callback).
Attachment #596059 - Attachment is obsolete: true
Attachment #596140 - Flags: review?(benjamin)
Attachment #596059 - Flags: review?(benjamin)
(Assignee)

Updated

5 years ago
Assignee: benjamin → joshmoz
Have you looked at the existing bugs related to unloading plugins? My main worry for this bug was that we'd cause those other bugs to appear by default. See bug 661260, especially comment 1, and also some of the commentary in bug 520639. I suspect that we should attempt to unload only OOPP plugins, and never in-process plugins.
(Assignee)

Comment 37

5 years ago
I think we should entirely remove the ability to unload any plugin from a non-plugin process. I don't even want to offer that option with a pref any more. I'll do that in an updated patch, and the unloadASAP pref will only apply to OOPP.
(Assignee)

Comment 38

5 years ago
Created attachment 596181 [details] [diff] [review]
delayed unload strategy, v2.0

Updated to never try to unload in-process plugin libraries. Only tested on Mac OS X so far.
Attachment #596140 - Attachment is obsolete: true
Attachment #596181 - Flags: review?(benjamin)
Attachment #596140 - Flags: review?(benjamin)
(Assignee)

Comment 39

5 years ago
Created attachment 596522 [details] [diff] [review]
delayed unload strategy, v2.1

Cancel the timer (if there is one) when a new instance is created.
Attachment #596181 - Attachment is obsolete: true
Attachment #596181 - Flags: review?(benjamin)
(Assignee)

Comment 40

5 years ago
We should probably fix bug 661260 before making plugin unloading much more likely for the average user.
Depends on: 661260
(Assignee)

Comment 41

5 years ago
try server run:

https://tbpl.mozilla.org/?tree=Try&rev=bfb36b156244
(Assignee)

Updated

5 years ago
Attachment #596522 - Flags: review?(benjamin)
(Assignee)

Comment 42

5 years ago
Created attachment 597036 [details] [diff] [review]
delayed unload strategy, v2.2

Updated to current trunk.
Attachment #596522 - Attachment is obsolete: true
Attachment #597036 - Flags: review?(benjamin)
Attachment #596522 - Flags: review?(benjamin)
Attachment #597036 - Flags: review?(benjamin) → review+
(Assignee)

Comment 43

5 years ago
pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/27a6f4156f0a
https://hg.mozilla.org/mozilla-central/rev/27a6f4156f0a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13

Comment 45

5 years ago
I don't see 'dom.ipc.plugins.unloadASAP' anywhere in about:config.  Is it something that has to be added manually or did something get missed?

Comment 46

5 years ago
(In reply to IU from comment #45)
> I don't see 'dom.ipc.plugins.unloadASAP' anywhere in about:config.  Is it
> something that has to be added manually or did something get missed?

I believe you have to create it by yourself, like old plugins.unloadASAP.

Firefox will unload plugin after it's unused 3 minute by default now.

Comment 47

5 years ago
Does this respond to memory pressure events (i.e. immediately unload on memory pressure)?
No.
You need to log in before you can comment on or make changes to this bug.