Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Fix WebExtension Excessive Memory Usage #12232

Closed

Description

@cschanaj

Type: other

Re-open of #1775, based on #1775 (comment) ordered by priority (personal)

GOAL

???

PROPOSAL

Proof of concept PRs, may or may not be get implemented for various reasons. Discussions should be made in their respective PRs.

TODO

Core

UI

PENDING FIX

WON'T FIX

Bugs or unexpected features found when reading through the code

Activity

  1. RReverser commented on Sep 1, 2017

    @RReverser
    Contributor

    Compile RegExp only when it is neccessary

    Not sure about SpiderMonkey, but in V8 regexps are anyway compiled lazily only on the first use and not when regexp literal is met or new RegExp is called.

  2. cschanaj commented on Sep 1, 2017

    @cschanaj
    CollaboratorAuthor

    Using the constructor function provides runtime compilation of the regular expression.

    Firefox only mention that new RegExp() is compiled in the runtime. Not really sure if it is lazily compiled or not. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions

    @RReverser do you have a source confirming for V8's behavior? Confirmed this behavior on Chromium browser using the console and taking heap snapshot. It also seems to me that Firefox does lazy compilation as well. thanks!!

  3. cowlicks commented on Sep 2, 2017

    @cowlicks
    Contributor

    RE: radix trie with reversed hostnames. I've been wanting to use something like this for privacy badger's storage datastructure. I have a pending PR with an implementation here https://github.com/EFForg/privacybadger/pull/1568/files#diff-930ddd92cefc0f8bc3a8722e10b3e05e

    I'm not using Map, but I probably should be.

    cc @Hainish

  4. Bisaloo commented on Sep 2, 2017

    @Bisaloo
    Collaborator

    Several of those points will require a major rewrite of google rulesets (duplicated multiple times, right wildcards, etc.). I don't really like the way targets and rules are split right now anyways. I was already thinking of a major overhaul of these rulesets. I will probably open a new issue about it soon.

  5. cschanaj commented on Oct 6, 2017

    @cschanaj
    CollaboratorAuthor

    It seems that Gorhill has disabled the "Issues" in his repo. Here is some comments made by Gorhill that we might be interested in

    18 Jun 2014

    The status of the issue:

    https://code.google.com/p/chromium/issues/detail?id=320723

    is ambiguous. It's not marked as fixed "verified", on the other hand it's labeled M32. Quite unclear whether the memory leak was solved or not.

    In any case, there are many places in HTTPSB where using a long-lived connection makes more sense than using a one time message, which comes with a good overhead if I read correctly.

    Note: It is marked as fixed "verified" AND labeled M32 now.

    18 Jun 2014

    Well that is interesting.. Not using chrome.runtime.sendMessage() reduces markedly memory footprint of the loaded web page when loading the vim test page, and it does appear it is easier on the CPU -- so finding is significantly less overhead using Port.postMessage() than chrome.runtime.sendMessage().

    18 Jun 2014

    Yes, but it's not as simple as replacing one string with another. I need to write my own sendMessage()-like code, but instead of creating a port every time like the chrome API does, I reuse the one instance.

    20 Jun 2014

    Finally got rid of chrome.runtime.sendMessage() from everywhere, now using ports. For each message sent, chrome.runtime.sendMessage() creates a port, so this is an improvement. I can't measure for sure though. If there are still memory leaks with using sendMessage() as reported in the above-linked chromium issue, then they should be fixed as far as HTTPSB is concerned.

    I did notice in some previous memory tests using heap snapshot diffs that there seemed to be a lot of instances of objects which I believe might have been related to message event objects.

    In any case, even if sendMessage() wasn't leaking, using a longer-lived port rather than one-per-message is saner. Currently ports are created when an extension page or a content script is created, and the same port is used until the page or content script is destroyed. Much less overhead.

  6. Hainish commented on Nov 30, 2017

    @Hainish
    Member

    The most common use of chrome.runtime.sendMessage() in the codebase currently is to communicate which rulesets are active in a tab to the popup, upon creation. Other usages include communicating options toggles, importing a json settings blob from the options UX, as well as adding and deleting new rules. These currently all must necessarily go through sendMessage since addon HTML pages have to communicate with the background context.

    Profiling the extension memory usage after subsequent clicks on the popup after visiting boingboing.net (which has lots of active rulesets communicated), I do see an increase overall, but I'm unsure what to look for in a heap analysis to see how much is due to this particular leak. And the overall memory usage in Chrome vacillates for me between 75 and 86M, settling at 72M.

    I wonder if we can safely ignore this problem until the upstream leak is patched. It seems to me that any minimization we do with sendMessage will come at a cost of usability or presentation to the user of ruleset data.

  7. jhpratt commented on Feb 4, 2019

    @jhpratt

    Any updates on this? A quick check by toggling this extension shows that it's using between 40 and 80 MB of memory (uncertainty because it's not the only extension).

  8. jhpratt commented on Jul 6, 2019

    @jhpratt

    @Giltyhub2 I actually saw that pull elsewhere and reinstalled — I imagine wasm will help out as well. I'll certainly report back if I notice any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions