Develop experimental WebExtension API for getting Firefox's memory usage
Categories
(WebExtensions :: Experiments, enhancement, P3)
Tracking
(firefox71 affected)
Tracking | Status | |
---|---|---|
firefox71 | --- | affected |
People
(Reporter: mconley, Assigned: KrisWright)
References
Details
(Whiteboard: [experiments][triaged][dev-ux])
Attachments
(1 file)
Prior art that we might want to adopt and then leapfrog: https://developer.chrome.com/extensions/system_memory Additionally, note that a JSM is being developed in bug 1255843 which should add some useful shortcuts for getting memory measurements for content processes. This experimental API could perhaps use that JSM instead of talking to the nsIMemoryReporterManager directly. Use cases for this API include: 1) Getting notifications about things like memory pressure so that WebExtensions can attempt to clear caches 2) Adding UI to Firefox to report detailed memory usage statistics More futuristically, billm also talked at one point about a separate experimental WebExtension API for helping Firefox decide which content process to open particular tabs in for e10s-multi. Memory usage data might be useful for making such decisions in that theoretical API.
Comment 1•8 years ago
|
||
In case it's relevant: xpcom/base/nsIMemory.idl has some stuff related to memory pressure notifications.
Comment 2•8 years ago
|
||
Hi Nick, is someone on your team interested in doing something in this area?
Updated•8 years ago
|
Reporter | ||
Comment 3•8 years ago
|
||
Note that this was an idea that billm proposed in London, and I wanted to make sure it wasn't forgotten.
Comment 4•8 years ago
|
||
(In reply to :shell escalante from comment #2) > Hi Nick, is someone on your team interested in doing something in this area? I'm not 100% sure this really was intended to be for me, but in case it was: sorry, not at the moment.
Updated•7 years ago
|
Comment 5•7 years ago
|
||
I put together a draft of a WebExtension experiment at: https://github.com/EricRahm/memory-api-experiment It includes a `getInfo` method that's just a pass-through to `Memory.summary`, so that means it just includes rss and uss for all processes. What's nice is we can update Memory.jsm to add more info without messing the WebExtension API (I have a draft patch for adding ghost window counts). Addtionally it has an `onLowMemory` event that hooks into the internal 'memory-pressure' event. I explicitly didn't add 'total system memory' and 'system memory used' because a) we don't have a reporter for that currently and b) I'm not sure that'd pass the privacy test. mconley would you mind providing some feedback on what I have implemented or helping me find a more appropriate reviewer?
Reporter | ||
Comment 6•7 years ago
|
||
I like it - I also like that we're considering exposing more than the Chromium API. It gets a thumbs up from me - though you'll probably want to talk to the WebExtensions team in general to try to get this integrated. They're the deciders on what gets adopted as official APIs. Tentatively redirecting to :aswan to see if he has any feedback.
Comment 7•7 years ago
|
||
From a quick glance, this looks good, thanks! What's the longer-term plan here? Should we land this directly in central so that extensions can use it without having to install a separate experiment? (I or somebody else should take a more careful look through the implementation if we're going to do that, but from a high level there aren't any glaring problems)
Updated•6 years ago
|
Comment 8•6 years ago
|
||
I removed Memory.jsm in bug 1474990, here is a link to the changeset for future reference if you need to re-add it here: https://hg.mozilla.org/mozilla-central/rev/6ee592ecc6ac
Updated•6 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
Webextension api to get memory usage. Also brings back memory.jsm, as it is used by the api.
Assignee | ||
Comment 12•5 years ago
|
||
Comment on attachment 9079115 [details]
bug 1296898 - Memory api (pulled from erahm's experiment) as a part of the webextension api
I've never looked at the web extension code before so I followed the example set for me by other apis in this directory, but in writing the test file it seems that I forgot to do something to implement the api. For reference, I tried creating an extension like this to test:
let testExtension = ExtensionTestUtils.loadExtension({
background,
manifest: {
permissions: ["memory"],
}
});
And I got this warning, followed by a long list of permitted values/regexp ("memory" not included among them):
WARN Loading extension '{6f5bc79b-8d6d-4242-84ed-e100654e69be}': Reading manifest: Error processing permissions.0: Value "memory" must either: ...
And then after it couldn't be loaded, any attempts to read into "memory" are undefined. I'm assuming I missed something or did something wrong when I added the file. Is there anything else I need to add to make the api usable?
Comment 13•5 years ago
|
||
Comment on attachment 9079115 [details]
bug 1296898 - Memory api (pulled from erahm's experiment) as a part of the webextension api
Andrew should be better to get feedback from at the moment at least.
Updated•5 years ago
|
Comment 14•5 years ago
|
||
(In reply to Kris Wright :KrisWright from comment #12)
WARN Loading extension '{6f5bc79b-8d6d-4242-84ed-e100654e69be}': Reading manifest: Error processing permissions.0: Value "memory" must either: ...
Kris (M) covered this in his review:
https://phabricator.services.mozilla.com/D38536#inline-233381
Comment 15•5 years ago
|
||
Is this meant to be a privileged api, or a public api? If the former, you need to make sure the patch does that (it doesn't currently). If the latter, FYI :Fallen
Comment 16•5 years ago
|
||
IOW, the bug is titled that this is experimental, so be sure to make the permission privileged.
Assignee | ||
Comment 17•5 years ago
|
||
(In reply to Shane Caraveo (:mixedpuppy) from comment #16)
IOW, the bug is titled that this is experimental, so be sure to make the permission privileged.
Once the extension was made privileged, I got this fail from the test_ext_permissions.js xpcshell test:
Permission 'memory' intentionally granted without prompting the user - false == true
I'm guessing I need to update this list here: https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/toolkit/components/extensions/test/xpcshell/test_ext_permissions.js#562
And then have a WebExtension peer review it. Is that correct?
Comment 18•5 years ago
|
||
Yes, you will need to add it there. You should also include a small permission test specific to your api, there are several examples, here's one[1]
You can r? me for the extension api parts, and/or continue with :kmag who has already made review comments.
Comment 19•5 years ago
|
||
Hi Folks, thanks for making some progress here. I have a few product questions before we push this API:
- Given the target now seems to be privileged extensions only, what internal add-ons will be using this?
- What team will be on the hook for maintaining this API?
Going back to the use cases in comment 0, it sounds like this was originally targeted as a public API. If that is the case, with the current implementation I'm concerned that a way for extensions to trigger the memory pressure event would be used by a bunch of add-ons thinking they can use this to "save memory" in Firefox.
Exposing detailed information about the memory configuration to add-ons publicly, I'd like to make sure we have use cases for that. The obvious one would be to allow an add-on to show a memory manager of sorts, but I rather think this is where about:memory should shine. Are there any additional cases? A low memory event would make sense publicly.
If there are no specific add-ons using this and there isn't a strong case for a public API, I'd prefer if we could keep this an experiment and not push it to m-c. Developers interested in fiddling with this can include that experiment within their extension.
Comment 20•5 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #19)
Exposing detailed information about the memory configuration to add-ons publicly
Unlike the Chrome version, this explicitly does not expose system memory information, just Firefox's memory usage.
If there are no specific add-ons using this and there isn't a strong case for a public API, I'd prefer if we could keep this an experiment and not push it to m-c. Developers interested in fiddling with this can include that experiment within their extension.
This was originally intended as public, although I'd expect there to be a memory permission that needs to be granted. There's a use case in bug 1351346, I'd also imagine it would be useful for various tab unloading add-ons.
re: GC, mccr8 weighed in on this previously in bug 1351346, comment 2:
I understand your concern about forceGC/forceCC, but I think it is mostly harmless in practice. I don't know how useful it actually is, though. Somebody can trigger GC and CC from about:memory if they really want to, but it isn't something you'll be doing all of the time.
So it's probably fine. I can see it being useful for folks writing fuzzers and experimenting with different memory reduction heuristics (like triggering a GC before moving on to unloading a tab).
Comment 21•5 years ago
|
||
Pushed by kwright@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8e400446aedc Memory api (pulled from erahm's experiment) as a part of the webextension api r=mixedpuppy
Comment 22•5 years ago
|
||
bugherder |
Comment 23•5 years ago
|
||
I'm sorry I didn't respond earlier, this bug went off my radar. These questions are still open:
- Given the target now seems to be privileged extensions only, what internal add-ons will be using this?
- What team will be on the hook for maintaining this API?
I understand how the API could be useful, though I do not understand why it needs to be in-tree, especially now that this is a privileged API. The extension could just as well include it as an experiment API. If Mozilla is developing an add-on like the gecko profiler that would need this kind of API I would reconsider, but keeping it around just because it could be useful doesn't seem like a good idea to me.
Given what I know now I'd prefer if we could back this out and wontfix the bug, let me know if there is something I am missing.
Updated•5 years ago
|
Comment 24•5 years ago
|
||
Backed out changeset 8e400446aedc (bug 1296898) at dev's request.
Backout:
https://hg.mozilla.org/integration/autoland/rev/976a5aa452f9dbf252135ad1617c01d2fd1fd253
Comment 26•5 years ago
|
||
Thanks for the backout. I'm going to close this bug WONTFIX now, if there is an update on the questions from comment 23 I'm happy to reconsider.
Description
•