search

gorhill / httpswitchboard

Point & click to forbid/allow any class of requests made by your browser. Use it to block scripts, iframes, ads, facebook, etc.
GNU General Public License v3.0
1.33k stars 83 forks source link

Avoid using chrome.runtime.sendMessage() wherever possible #345

Closed gorhill closed 10 years ago

gorhill commented 10 years ago

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.

gorhill commented 10 years ago

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

jonvuri commented 10 years ago

Is there any more busywork to do replacing instances of it?

gorhill commented 10 years ago

Is there any more busywork to do replacing instances of it?

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.

gorhill commented 10 years ago

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.