Closed Bug 501485 Opened 11 years ago Closed 8 years ago

investigate new NPAPI plugin unloading strategy

Categories

(Core :: Plug-ins, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: jaas, Assigned: jaas)

References

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file, 4 obsolete files)

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.
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.
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
Duplicate of this bug: 662028
(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
Duplicate of this bug: 673356
The processes after closed takes a long time to release memory.
Firefox should terminate the process when you disable the plugin in the AOM.
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.
I wasn't responding to you.

This bug has nothing to do with flash or flash plugin container memory usage.
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.
Its not about the container taking a long time to release memory, thats a flash bug itself.
Blocks: 675471
Whiteboard: [MemShrink]
Duplicate of this bug: 675471
Whiteboard: [MemShrink] → [MemShrink:P2]
Duplicate of this bug: 688530
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
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
i can't seem to find a bug for that, is there one?
Duplicate of this bug: 711825
Depends on: 90268
Duplicate of this bug: 720235
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.
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.
>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?
Benjamin - do you mind if I take this bug or are you planning to work on it soon?
Attached patch delayed unload strategy, v1.0 (obsolete) — Splinter Review
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)
Attached patch delayed unload strategy, v1.1 (obsolete) — Splinter Review
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: 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.
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.
Attached patch delayed unload strategy, v2.0 (obsolete) — Splinter Review
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)
Attached patch delayed unload strategy, v2.1 (obsolete) — Splinter Review
Cancel the timer (if there is one) when a new instance is created.
Attachment #596181 - Attachment is obsolete: true
Attachment #596181 - Flags: review?(benjamin)
We should probably fix bug 661260 before making plugin unloading much more likely for the average user.
Depends on: 661260
Attachment #596522 - Flags: review?(benjamin)
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+
https://hg.mozilla.org/mozilla-central/rev/27a6f4156f0a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
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?
(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.
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.