-
Notifications
You must be signed in to change notification settings - Fork 325
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
Companion is always at [medium] in about:performance in Firefox #721
Comments
Thank you for noticing this. Companion is polling IPFS API every 3 seconds to refresh the peer count displayed on browser action badge and detect IPFS node going online/offline. IIRC we use We could increase IPFS API poll interval but it would impact UX around online/offline detection. Potential mitigation:
Thoughts? |
What is the use-case for seeing the peer count? What decisions are
companion users making that is based on seeing that number change?
…On Sun, May 19, 2019 at 1:27 PM Marcin Rataj ***@***.***> wrote:
Thank you for noticing this.
I made a quick check and it is caused by setInterval running in the
background page.
Companion is polling IPFS API every 3 seconds to refresh the peer count
displayed on browser action badge and detect IPFS node going
online/offline. IIRC we use setInterval instead of browser.alarms
<https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/alarms>
because Chrome hardcoded min interval to be 1 minute.
We could increase IPFS API poll interval but it would impact UX around
online/offline detection.
Potential mitigation:
- Remove peer count from browser badge to hide the fact we refresh
state
- Pick bigger polling interval (10s? 30s?) in 'idle' mode (when
browser action UI is hidden)
- Poll API every 2s when browser action UI is visible
Thoughts?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#721>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAMHNY5ENIQTTHPG45DYSDPWGZ35ANCNFSM4HNYQCNA>
.
|
It is an indicator of connectivity health:
User does not do much with this, but IPFS Companion makes various decisions based on peering state. Peer count >= 0 means API is online, which in turn enables features that require access to API (quick upload) or Gateway ("Automatic Mode" enables/disables redirect to local node based on connectivity state). |
In that case, the mitigation sounds good. The decision state is "connected"
or not, and that's conveyed in a reasonable enough way even at every 30s
(for now).
When the stack as a whole is faster, maybe there's argument for
near-real-time updates... but at that point we'll likely have a better way
of getting this information than polling.
I like the idea of using peer count to hint at participation level for good
feels, but it doesn't trump battery life.
Network requests on an interval are basically a "perfect storm" wrt battery
consumption. Do we not have any other options?
A couple of other thoughts, but maybe pre-optimizing:
* Trigger peer update when any part of Companion is interacted with, or we
detect webui being opened, etc? Maybe would reduce the (already small, even
with the mitigation) chance that connection state is out of sync.
* Instead of our own timer, use idle interval callbacks so that we never
eat battery without reason:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/idle
|
I've been thinking about "triggering peer update when any part of Companion is interacted with" and we may be able to make it pretty "lazy":
Does it sound sensible? |
Nice to have: Some instrumentation so we can check different before/after these changes. |
Here because I just hit the same issue. Of note, I'm seeing this with "all ipfs integrations" set to off. I'm a big plus 1 of removing the peer count from the badge. I think we should generally reduce the prominence of how many peers you are connected to; it's doesn't tell you if you're going to have a good time finding the content you want. Zero peers means you are going to have a bad time, so we could special case that, but... for companion, I think it's way more interesting to focus the badge on "hey! this page comes in IPFS flavour", when we lazily check for a dnslink after letting the http request have a go... #710 |
Related: Firefox 76 will ship with a new profiler marker to include information about the webRequest blocking handlers registered by extensions (Bug 1625006). |
This seems to no longer be the case since we switched to Manifest V3 (#666). If you see perf. issues with 3.x version, please open new issue. |
The text was updated successfully, but these errors were encountered: