501485 - investigate new NPAPI plugin unloading strategy

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Josh Aas]

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.

Comment 1

[(not currently active) Ted Mielczarek]

There's also a memory pressure notification that stuart added, which would be a good time to free any unused plugins.

Comment 2

[Jo Hermans]

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.

Comment 4

[u88484]

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

Comment 10

[dindog]

(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)

Comment 12

[Himanshu]

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

Comment 13

[Danial Horton]

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

Comment 14

[Himanshu]

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

[Danial Horton]

I wasn't responding to you.

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

Comment 16

[Himanshu]

Then why my bug Bug 673356 has been marked as a duplicate of this bug.

Comment 17

[Boris Zbarsky [:bzbarsky]]

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

[Danial Horton]

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

Comment 21

[Angelo Borsotti]

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).

Comment 25

[Benjamin Smedberg]

I'm going to take this, but I'll want plugin-as-content to land first otherwise the merge will be difficult.

Comment 26

[Danial Horton]

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

Comment 27

[Benjamin Smedberg]

bug 90268

Comment 30

[sworddragon2]

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

[Danial Horton]

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

[sworddragon2]

>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?

Comment 33

[Josh Aas]

Benjamin - do you mind if I take this bug or are you planning to work on it soon?

Comment 34

[Josh Aas]

Attachment: 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.

Comment 35

[Josh Aas]

Attachment: delayed unload strategy, v1.1

Fixes a minor bug that probably had no real consequences (incorrect return value from timer Notify callback).

Comment 36

[Benjamin Smedberg]

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.

Comment 37

[Josh Aas]

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.

Comment 38

[Josh Aas]

Attachment: delayed unload strategy, v2.0

Updated to never try to unload in-process plugin libraries. Only tested on Mac OS X so far.

Comment 39

[Josh Aas]

Attachment: delayed unload strategy, v2.1

Cancel the timer (if there is one) when a new instance is created.

Comment 40

[Josh Aas]

We should probably fix bug 661260 before making plugin unloading much more likely for the average user.

Comment 41

[Josh Aas]

try server run:

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

Comment 42

[Josh Aas]

Attachment: delayed unload strategy, v2.2

Updated to current trunk.

Comment 43

[Josh Aas]

pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/27a6f4156f0a

Comment 44

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/27a6f4156f0a

Comment 45

[IU]

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

[dindog]

(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

[Daniel C]

Does this respond to memory pressure events (i.e. immediately unload on memory pressure)?

Comment 48

[Benjamin Smedberg]

No.