facebook-political-ads
facebook-political-ads copied to clipboard
Extension Performance
Readers have complained that they some times get a Firefox warning that the plugin is blocking the webpage with an stop script option. I've personally noticed that opening the popup in Chrome takes a few seconds after using it for a while.
Things to look into:
- large local data set leading to slow initialisation
- loading too many languages
- too aggressive facebook.com DOM access
Would be nice to solve this with an TTD approach and keeping track of performance over time.
Yep, this is a hard problem right now because of how the parser is written. I think it arises here: https://github.com/propublica/facebook-political-ads/blob/master/extension/src/parser.js#L221 Which was my way of figuring out how to grab the ad endpoint from facebook's popup. I'd love it if you could take a little time to see if we can scope that selector better so as not to pick up so many changes.
I also think we need to decouple all of the promises in that file so we can test individual functions, which will help with identifying the performance issues/
As far as the chrome thing, I think it has to do with localStorage, and I try and limit the amount of ads we store there to 100, but it still seems too much: https://github.com/propublica/facebook-political-ads/blob/master/extension/src/utils.js#L50
We could drop it to 20, because it isn't all that important to store all the ads.
Does this sound like something you could help out with?
Yep, happy to work on this. Thanks for the pointers. I can't promise anything time wise, heading out for vacations mid next week. I'll post here once I'm actively working on it and making progress.
I've pushed a hotfix here: https://github.com/propublica/facebook-political-ads/commit/69d8324f73a6de53403d0dfa8b06d3fc6b57452e
@jeremybmerrill pointed out that we weren't successfully cleaning up all of the observers. Still, I'd love to rework parser.js into something more sane. I'm going to keep this open as a tracking issue.
FYI, I'm currently working through a large refactor to make it easier to test the extension, so hold off for now. I'll have something to show tomorrowish.
Ha, cool. I had just started looking into the slowness of the popup, but I'll hold off.
Found this, but am not sure it's our problem. https://stackoverflow.com/questions/26276815/my-chrome-extension-popup-opens-after-a-few-seconds-its-slow-compared-to-other
this is worth a shot in the meantime: https://stackoverflow.com/a/30164224
Yep, that's what I'm gonna try. I guess the idea is that it fools Chrome into leaving the extension running.
That’d be dumb. We still need to figure out startup performance though.
On Fri, Mar 16, 2018 at 7:41 AM Jeremy B. Merrill [email protected] wrote:
Yep, that's what I'm gonna try. I guess the idea is that it fools Chrome into leaving the extension running.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/propublica/facebook-political-ads/issues/41#issuecomment-373734091, or mute the thread https://github.com/notifications/unsubscribe-auth/AADYRRAGZLCo45YIJcRnNZsZ3sgi7rnyks5te88IgaJpZM4SPIwJ .
I'm gonna give it a try. Computers do dumb things sometimes.
It may be worthwhile to get a lighter version of things rendered, then go get ads from teh server, etc. so something happens right away when you click (even if it's just a loading spinner). Excited to see your refactor.
That’s basically how it works now. I think we’re being blocked on local storage.
On Mar 16, 2018, at 7:50 AM, Jeremy B. Merrill [email protected] wrote:
It may be worthwhile to get a lighter version of things rendered, then go get ads from teh server, etc. so something happens right away when you click (even if it's just a loading spinner). Excited to see your refactor.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.
var ls_start_time = new Date();
persistedState = deserialize(localStorage.getItem(key));
finalInitialState = merge(initialState, persistedState);
console.log("loaded state from localStorage in ", new Date() - ls_start_time);
It's consistently taking -- on a fresh start of Chrome -- less than 20ms to read from localStorage, usually 4-8ms. With 20 ads stored in there. I don't think localStorage is the problem.
It does take about 150ms to get to the point where it's requesting ads from the server. While that's kind of a lot, impressionistically, the delay is more like 5 seconds.