Closed Bug 1296898 Opened 5 years ago Closed 2 years ago

Develop experimental WebExtension API for getting Firefox's memory usage

Categories

(WebExtensions :: Experiments, enhancement, P3)

enhancement

Tracking

(firefox71 affected)

RESOLVED WONTFIX
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.
In case it's relevant: xpcom/base/nsIMemory.idl has some stuff related to memory pressure notifications.
Hi Nick, is someone on your team interested in doing something in this area?
Flags: needinfo?(nchapman)
Priority: -- → P2
Whiteboard: [experiments] triaged
Flags: needinfo?(nchapman) → needinfo?(n.nethercote)
Note that this was an idea that billm proposed in London, and I wanted to make sure it wasn't forgotten.
Severity: normal → enhancement
(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.
Flags: needinfo?(n.nethercote)
See Also: → 1351346
Priority: P2 → P3
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?
Assignee: nobody → erahm
Status: NEW → ASSIGNED
Flags: needinfo?(mconley)
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.
Flags: needinfo?(mconley) → needinfo?(aswan)
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)
Flags: needinfo?(aswan)
Product: Toolkit → WebExtensions
See Also: → 1474990
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
Whiteboard: [experiments] triaged → [experiments][triaged][dev-ux]
Duplicate of this bug: 1351346

I'll see if we can revive this.

Assignee: erahm → kwright

Webextension api to get memory usage. Also brings back memory.jsm, as it is used by the api.

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?

Attachment #9079115 - Flags: feedback?(kmaglione+bmo)

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.

Attachment #9079115 - Flags: feedback?(kmaglione+bmo) → feedback?(aswan)
Attachment #9079115 - Flags: feedback?(aswan)

(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

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

Flags: needinfo?(philipp)

IOW, the bug is titled that this is experimental, so be sure to make the permission privileged.

https://searchfox.org/mozilla-central/rev/4436573fa3d5c4311822090e188504c10c916c6f/toolkit/components/extensions/Extension.jsm#148

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

https://searchfox.org/mozilla-central/rev/4436573fa3d5c4311822090e188504c10c916c6f/toolkit/components/extensions/Extension.jsm#148

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?

Flags: needinfo?(mixedpuppy)

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.

[1] https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/toolkit/components/extensions/test/xpcshell/test_ext_networkStatus.js#174

Flags: needinfo?(mixedpuppy)

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.

Flags: needinfo?(philipp)

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

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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

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.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: needinfo?(philipp)
Target Milestone: mozilla71 → ---

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.

Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Flags: needinfo?(philipp)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.