-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Very slow performance on page with Facebook Container extension enabled #601
Comments
I just noticed a super slow performance on Github pull request page with the addon enabled.
|
I suspect this has to do with memory usage. I notice this behavior on pages that use 150+ MB (as shown in about:performance). DOM updates with Facebook Container enabled take much longer. It's not so much the amount of DOM nodes, however. I tested with a page that uses a lot of data: URL's for images, and with the same page where those URL's are shorter (using URL.createObjectURL). The amount of DOM nodes is the same, but the memory consumption much lower. Facebook Container makes the inoptimized version almost unusable, while the low-memory version works fine with FBC enabled. |
While playing with the source in the add-on debugger I discovered that What is the thought behind doing this on every click? Maybe there's a way to listen for changes to the DOM and only running the patternDetection function against the changed parts? |
Hello again. I've created a patch that uses I think my code might need some polishing. Will you accept a PR? |
The inital goal for the content script on non-FB pages was to badge Facebook share/login buttons across the web. Often times, these buttons would show up after a user-click (Example: Clicking the "login" button in a nav triggers a modal with a "Login with Facebook" button). The constant checking was to catch these. Using Thanks for digging in on this @mrwensveen! |
Oh - One thing I should add. We have a draft PR to add a settings panel to this add-on. (#721) One of the optional settings we're adding is turning off the badge detection entirely. Because that setting is optional, I beleive fixing the memory consumption is still the best/right course of action. |
We've run into this issue as well with an application we're building -- with Facebook Container enabled, performance falls completely off a cliff. Disabling the addon fixes things. My investigations match @mrwensveen's observations -- it seems as though there's a Admittedly the context that this is happening in has a huge number of DOM nodes, but it makes the entire thing unusable. Is there anything I can do to help getting this PR through and an update to the addon released? With regard to #721, is it feasible for a site to somehow opt-out of this badge detection, on the basis that it won't be necessary? I don't believe this would undermine the utility of the addon itself, although I admittedly am not an expert. |
This is still an issue. Heavy websites have very degraded performance when this extension is enabled. Why doesn't the extension inspect request links instead of looking at the DOM (which is costly)? It'd make more sense to block an outgoing request to Facebook tracking instead of inspecting DOM elements IMHO. On this page specifically with the extension enabled, trying to change filters adds a huge overhead the average that drops the average FPS to less than 10. In contrast, with the extension disabled the average FPS remains above 30 FPS. This gets worse in interactive websites. I'm currently debugging on Firefox 88.0.1 (64-bit) latest. Proposed SolutionI recommend using background scripts to inspect requests instead of looking at the DOM, which would make up for a huge performance improvement. This could affect the user experience since it'd completely disable the "Facebook Container" badge on links and buttons, but is fixable. Instead of actively scanning the DOM for changes, you could use the mousemove event to detect hover over links and buttons and then apply existing UI logic such as adding the badge. |
Still an issue. I can't even open up a message on firefox anymore because it goes on an endless loading binge. Loading posts, loading the chats list and so on. |
It seems my PR has some merge conflicts now (it hadn't when I created it). I'll try to take a look at it. It may help to get this through. |
I fixed a few merge conflicts. Should be okay now. Please merge my PR, @maxxcrawford 🙏 |
When using the accordions on the page https://developer.mypurecloud.com/api/rest/v2/conversations/index.html, the web page freezes with Facebook Container enabled. With the extension disabled, the page is relatively smooth.
Actual behavior
Web page is very slow. Profile: https://perfht.ml/38HAxnk
Expected behavior
Expected web page to be smooth, which it is with the Facebook Container extension disabled.
Steps to reproduce
GET /api/v2/conversations/messages/{conversationId}/messages/{messageId}
accordion item)The text was updated successfully, but these errors were encountered: