Skip to content
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

Reconnect spinner burns CPU #8068

Closed
benzea opened this issue Jun 13, 2020 · 41 comments
Closed

Reconnect spinner burns CPU #8068

benzea opened this issue Jun 13, 2020 · 41 comments
Labels

Comments

@benzea
Copy link

benzea commented Jun 13, 2020

Steps to reproduce

  1. Disconnect network
  2. Look at telegram or look at top/strace

Or, said differently, try to work somewhere without internet connection (and power supply).

Expected behaviour

Telegram should not burn CPU. In particular when it is not even visible (e.g. different workspace). Which it probably only can do because it is using X11 (Xwayland) rather than wayland (GNOME shell).

Easiest fix would likely be to just do a countdown that is only updated e.g. every 5 seconds instead of showing a spinner. At least when the window is not focused.

Configuration

Operating system: F32

Version of Telegram Desktop: 2.1.11

Installation source (Linux Only) - flatpak

Used theme: default

@p4vook
Copy link

p4vook commented Jun 18, 2020

Reproducable on Gentoo GNU/Linux, Telegram 2.1.7, installation source - Gentoo package, DE - Plasma Wayland.

@benzea
Copy link
Author

benzea commented Jun 18, 2020

I am also seeing a lot of wakeups while just running and connected. This could be related if QT has a similar behaviour to GTK where spinners may cause wakeups (but no rendering) if they are not stopped in certain use cases.

@stale
Copy link

stale bot commented Oct 23, 2020

Hey there!

This issue will be automatically closed in 7 days if there would be no activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

@stale stale bot added the stale label Oct 23, 2020
@benzea
Copy link
Author

benzea commented Oct 23, 2020

Behaviour is still there.

@themighty1
Copy link

Still present on Ubuntu 20.04 when network connection goes down. Telegram uses 6-7 % CPU.
Oftentimes something "snaps" all Telegram starts using 100% CPU. Even when I go back online, the CPU usage stays at 100%. The only remedy is to restart Telegram.

@sh4dowb
Copy link

sh4dowb commented Apr 16, 2021

I literally woke up last night to the sound of my cpu because my internet went offline. it also uses ALL cores to make sure it is annoying enough. thanks telegram.

@stale
Copy link

stale bot commented Oct 13, 2021

Hey there!

This issue was inactive for a long time and will be automatically closed in 30 days if there isn't any further activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

@stale stale bot added the stale label Oct 13, 2021
@benzea
Copy link
Author

benzea commented Oct 18, 2021

Still the same.

@stale stale bot removed the stale label Oct 18, 2021
@stale
Copy link

stale bot commented Apr 17, 2022

Hey there!

This issue was inactive for a long time and will be automatically closed in 30 days if there isn't any further activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

@stale stale bot added the stale label Apr 17, 2022
@benzea
Copy link
Author

benzea commented Apr 17, 2022

Still the same.

@stale stale bot removed the stale label Apr 17, 2022
@github-actions
Copy link

Hey there!

This issue was inactive for a long time and will be automatically closed in 30 days if there isn't any further activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

@github-actions github-actions bot added the stale label Oct 15, 2022
@benzea
Copy link
Author

benzea commented Oct 24, 2022

still a problem

@github-actions github-actions bot removed the stale label Oct 25, 2022
@pjrobertson
Copy link

Also a problem for me on macOS 12

@VahidSh69
Copy link

I just updated Telegram Desktop to v4.6.10b, and the problem persists.

@shtirlic
Copy link

shtirlic commented May 17, 2023

Still the same behavior, also it some times cause my laptop to overheat and freeze, so I have to close tg when I am away from laptop.

@themighty1
Copy link

@shtirlic , does this behaviour happen when your network connection goes down?

@shtirlic
Copy link

shtirlic commented May 17, 2023

@themighty1 actually the debug process is quite hard for this issue, my scenario is like this: I lock my laptop with tg running in the background and in the morning I got frozen and hot laptop with all fans on max, in journalctl I do not see any suspicious errors or something bad around the projected freeze time, but for sure I saw iwd reconnection messages some time before. It might be that lost connection is not instantly spinning CPU to orbital speed just in time of disconnection, but somehow it triggers some deadlock in tg code a little bit later. Reconnection/Lost connection high CPU issues I have seen for years before, I saw this on Windows and macOS too. Currently my setup is: Arch Linux with KDE(x11). I will try to gather more info using audit logs and logging system usage/thermal/network activity.

@themighty1
Copy link

Thanks for the description.
I know it's a tall call but someone motivated enough could compile a debug version of tgdesktop and run it under a CPU profiler to see which function is responsible for the CPU loop.

@pjrobertson
Copy link

I know it's a tall call but someone motivated enough could compile a debug version of tgdesktop and run it under a CPU profiler to see which function is responsible for the CPU loop.

I just looked in to doing this, but the build instructions don't work for Mac. For other devs, this is incredibly easy to reproduce for me on macOS:

  1. Open Telegram
  2. Go to System Preferences > Network > Advanced ... and set up a SOCKS proxy for an IP/port that hangs.
  3. See CPU spikes

@benzea
Copy link
Author

benzea commented May 17, 2023

Original reporter here. As I implied in my original report, the problem appears to be excessive redrawing. Just plain redrawing of a spinner ...

i.e. powertop says:

                Usage       Events/s    Category       Description
              4.6 ms/s      72.9        Process        [PID 967096] telegram-desktop --

and well, yeah, redrawing with 60Hz would be consistent with that. sysprof shows the CPU time is pretty much all spend drawing too:
Screenshot from 2023-05-17 13-35-21

So, really, just replace that reconnection spinner with something that is only updated once every few seconds at max.

And yes ... it could be that the desktop environment or QT is partially to blame here (e.g. window shouldn't actually get redrawn if it is completely obscured). But, maybe it would still be nice to solve it here in some way.

@benzea
Copy link
Author

benzea commented May 17, 2023

Sorry, better screenshot of the sysprof data:
image

@yanovich
Copy link

@benzea, Thanks for pointing out the animation. The app does some work even when interface animation is disabled in settings.
Here is the capture of sysprof.

2023-06-21_capture.zip

Screenshot from 2023-06-21 19-28-37

@yanovich
Copy link

The problem is CPU usage offline is higher than online. CPU usage is 0% of a CPU thread online, 25% offline w/ animation on, 4% w/ animation off for i5-11600KF. and 0/50/5 for i5-8265U. 5% CPU usages is a big load on laptop battery. It consumes as much energy as watching a movie, but doing nothing.

@benzea
Copy link
Author

benzea commented Jun 22, 2023

@yanovich oh, true. And in that case it seems to be mostly polling and killing QT timers. So, I guess stopping/starting some timer somewhere ... outch

Copy link

Hey there!

This issue was inactive for a long time and will be automatically closed in 30 days if there isn't any further activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

@github-actions github-actions bot added the stale label Dec 20, 2023
@themighty1
Copy link

still an issue on the latest tg desktop I still see some ~5-10% CPU usage like I reported 2 years ago.

@sh4dowb
Copy link

sh4dowb commented Dec 20, 2023 via email

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Jan 12, 2024

There's a bug in the Qt 6 RHI abstraction that it mis-uses QImage's CoW feature and causes a detach (copy) of entire window buffer on every repaint so the animation is perhaps consuming way more CPU on Wayland than on Windows/macOS/X11. I've patched Qt for official builds in 4.14.4 so it should consume not more than official builds for other systems. Can't help distro builds though.

@joecool1029
Copy link
Contributor

Can't help distro builds though.

Can you post your qt patch? That would help...

@ilya-fedin
Copy link
Contributor

Currently it consists of 4 patches:
https://github.com/desktop-app/patches/blob/master/qtbase_6.6.1/0029-fix-backing-store-rhi-unneeded-copy.patch
https://github.com/desktop-app/patches/blob/master/qtbase_6.6.1/0030-fix-backing-store-rhi-xcb-unneeded-copy.patch
https://github.com/desktop-app/patches/blob/master/qtbase_6.6.1/0031-fix-backing-store-opengl-subimage-unneeded-copy.patch
https://github.com/desktop-app/patches/blob/master/qtwayland_6.6.1/0009-fix-backing-store-rhi-unneeded-copy.patch
But I think it could be made down to 2 patches: the fix-backing-store-opengl-subimage-unneeded-copy one + just remove the image.detach() from QBackingStoreDefaultCompositor::toTexture. This means less changes but fixing for all platforms at once (all Qt platforms affected but the current variant fixes only X11 and Wayland).

@mark-herbert42
Copy link

I am just curious - is this idiotic spinner so needed and wanted feature? It shits the bed 4 years now, and telegram still waiting till Qt do some updates. Is'nt it easier just remove it from tdesktop code? Simply do not paint it. I bet nobody will miss this piece of ....

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Mar 22, 2024

I am just curious - is this idiotic spinner so needed and wanted feature?

It's apparently invented by the designers and so developers have no right to remove it. Given that developers are the only one who read this bugtracker (it's a developer's idea to have it, it's not really official), no one who can decide on spinner removation or redesign read this.

@mark-herbert42
Copy link

As usual - marketing is the source of 99% of sht. I got you bro - will patch this ugly piece of sh* out from my personal build, that's what opensource is for. And my patch will serve me well for some years in advance - I know marketing design people, the worst decision they make is the Holy Cow for them, so code will remain for ages. Lets recall general Custer last words :)

@john-preston
Copy link
Member

@mark-herbert42 There is an option remove it, disabling the animations in Settings.

@p4vook
Copy link

p4vook commented Mar 22, 2024

Perhaps, an option to remove the spinner only and keep all the other animations could be useful

@mark-herbert42 There is an option remove it, disabling the animations in Settings.

@mark-herbert42
Copy link

Perhaps, an option to remove the spinner only and keep all the other animations could be useful

@mark-herbert42 There is an option remove it, disabling the animations in Settings.

That's exactly what I mentioned in my todays bug report - add option to remove just this ugly spinner somewhere in advanced options. Marketing sh*theads will never find it - it beyond their brain power, but linux users sure will.

@benzea
Copy link
Author

benzea commented Mar 22, 2024

It's apparently invented by the designers and so developers have no right to remove it.

One should never give any particular group privileges to do whatever they like. Yes, developers shouldn't just ignore designers, but the fix for that isn't to just do whatever the designers want. If the designers are unable to work with the existing constraints and find a solution, then seriously, just override them.

There are a few possibilities here:

  1. The designers haven't been made aware, but would provide a solution if they were aware of the problem.
  2. The designers refuse to think about it
  3. The designers know about it, but refuse to do anything

In case 1., make them aware. In case 2./3. they are either just overworked or lazy or incompetent. In those later cases, developers have plenty of reason to override them.

@ilya-fedin
Copy link
Contributor

If you want to make sure designers made aware, report the problem via official sources (e.g. in-app support).

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Mar 22, 2024

The only things developers can do is to optimize the behavior without changing how it's presented and that was already done after the @benzea's and @yanovich's profiling results. They can't do user-visible changes on their own or they would get a reprimand I guess. The only way to get a UI change is to report via support, having them to actually make the managers aware and the managers to do the decision. Or, well, you can always create a fork where you can override whoever you want.

btw one of @benzea's findings was that it works at 60Hz while should work at 120Hz, there was an attempt to fix this as well, I hope it was successful and Wayland has now animations as smooth as on other platforms.

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Sep 7, 2024

Telegram should not burn CPU. In particular when it is not even visible (e.g. different workspace). Which it probably only can do because it is using X11 (Xwayland) rather than wayland (GNOME shell).

The static binary and snap package use Qt version with surface suspension support now (5.4.5+). Assuming your compositor implements it properly and suspends the surface when it's not visible, this shouldn't be a problem anymore. Otherwise it might be the time to open an issue on the compositor side.

@Aokromes Aokromes closed this as completed Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests