Opened on 02/03/2019 at 12:41:41 PM
Closed on 02/04/2019 at 02:56:12 PM
Last modified on 02/25/2019 at 01:29:52 PM
#7257 closed change (fixed)
Throttle badge updates
| Reported by: | mjethani | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-3.5-for-Chrome-Opera-Firefox |
| Module: | Platform | Keywords: | |
| Cc: | sebastian, kzar | Blocked By: | |
| Blocking: | #7000 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by mjethani)
Background
Despite the changes in #7253, the extension still does a lot of unnecessary and avoidable updating of the badge. In this case, it is the badge text, which is updated every time there is a filter hit. The fact is that a lot of filter hits happen too close together such that the human eye could barely perceive the update to the text, assuming the browser even rendered it. The other fact is that a lot of filter hits happen in background tabs (especially when a link is opened in a new tab that is kept in the background) and tabs in other browser windows that are out of focus.
We could do a couple of things to minimize CPU usage in such cases:
- Throttle updates to the badge text at a lower refresh rate. In my tests, 4 fps (frames per second) seemed OK and did not feel like it was too slow.
- Update the badge text for only the active tab in the focused window; alternatively, update it for all active tabs in all windows.
I have written a patch that does both of the above. In order to test it, create the following HTML document:
<!DOCTYPE html>
<html>
<body>
<script>
for (let i = 1; i <= 100; i++)
setTimeout(() => fetch("https://example.com/test"), i * 50);
</script>
</body>
</html>
Then load the extension, open Chrome DevTools for the background page, go to the Performance tab, and start profiling. Then run the following Bash script to open the same document in 10 tabs:
#!/bin/bash for i in $(seq 10); do open -a 'Google Chrome' test.html done
After about 15 seconds, stop the profiler. If you look at the Summary tab, you'll find that the extension spends significantly less time executing script code with the patch applied. On my machine it cut the CPU usage in half.
What to change
In lib/stats.js:
- Keep track of all the active tabs and the windows to which they belong.
- When there's a filter hit, instead of updating the badge right away, schedule an update at a timeout of 250 ms (4 fps); on timeout, update all active tabs with their values from blockedPerPage.
Hints for testers
After this change, the badge will be updated for only any active tabs. There should be no visible difference in behavior to the user. In order to ensure that this is working as intended:
- Open two windows and two tabs in each of the windows; load some website where there are a number of filter hits (e.g. nytimes.com) in each tab; then switch between the tabs/windows and make sure that every time a tab becomes active it gets updated with the latest number of hits for the tab.
- Close one of the windows and repeat the above; make sure there are no errors in the background page console.
- Close all windows and make sure there are no errors in the background page console.
- Open one window and repeat the steps in 1; again, make sure the badge is updated correctly and there are no errors in the background page console.
- Turn off the "Show number of ads blocked in icon" option on the options page, reload the page, and make sure the number of hits is no longer shown
- Turn on the "Show number of ads blocked in icon" option on the options page, reload the page, and make sure the number of hits is shown now
Attachments (0)
Change History (12)
comment:4 Changed on 02/03/2019 at 01:13:50 PM by mjethani
- Cc sebastian kzar added
- Component changed from Unknown to Platform
comment:5 Changed on 02/03/2019 at 01:14:34 PM by mjethani
comment:6 Changed on 02/04/2019 at 08:19:21 AM by abpbot
A commit referencing this issue has landed:
Issue 7257 - Throttle badge updates
comment:9 Changed on 02/04/2019 at 02:55:15 PM by mjethani
- Owner set to mjethani
comment:10 Changed on 02/04/2019 at 02:56:12 PM by mjethani
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Resolution set to fixed
- Status changed from new to closed
comment:11 Changed on 02/05/2019 at 12:42:12 AM by sebastian
- Priority changed from Unknown to P2
- Ready set
comment:12 Changed on 02/25/2019 at 01:29:52 PM by ukacar
- Verified working set
If we can get agreement on this, I'd like to get this into ABP 3.5. It will make a difference to the benchmarks.