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

(tracking) Brave News Tab Helper causes high cpu parsing feeds on Tab navigation #30898

Closed
petemill opened this issue Jun 7, 2023 · 3 comments · Fixed by brave/brave-core#17923
Assignees
Labels
feature/brave-news formerly brave-today OS/Android Fixes related to Android browser functionality OS/Desktop perf QA/No release-notes/exclude

Comments

@petemill
Copy link
Member

petemill commented Jun 7, 2023

Description

As per #29125 it seems like the root cause of high cpu on some sites is parsing all the rss feeds found on any page. Some feeds are large and take some cpu time to parse. This is to power the "Follow this source in 1-click" feature:
image

As a result, after verifying this is the issue, or regardless, we should consider:

  • Only check for and parse the feeds if brave news is enabled, since the "follow this source" UI won't be shown anyway
  • Fetch the elements, but don't parse them until the user opens the panel (and show loading indicator whilst open). With this, we'll still be able to know to show the icon and the followed/unfollowed state since we'll have the feed URLs.
  • To get the feed title, perhaps don't parse the whole feed, but just enough to find the title element.

Thanks to @arthuredelstein for figuring out the potential (and probable) cause of #29125 and thinking about the solution.

Steps to Reproduce

Actual result:

Expected result:

Reproduces how often:

Desktop Brave version:

Android Device details:

  • Install type (ARM, x86):
  • Device type (Phone, Tablet, Phablet):
  • Android version:

Version/Channel Information:

  • Can you reproduce this issue with the current release?
  • Can you reproduce this issue with the beta channel?
  • Can you reproduce this issue with the nightly channel?

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:

@petemill
Copy link
Member Author

petemill commented Jun 12, 2023

Since the speed improvements of brave/brave-core#17923 is so great, perhaps the only thing left to do is

  • Only check for and parse the feeds if brave news is enabled, since the "follow this source" UI won't be shown anyway

What do you think @fallaciousreasoning ?

@brave-builds brave-builds added this to the 1.52.x - Release #5 milestone Jun 12, 2023
@fallaciousreasoning
Copy link

That sounds about right 😄

@petemill
Copy link
Member Author

I've opened #31019 to track disabling the feed finding when Brave News is completely disabled.

We can close this as a duplicate of #29511 as it solved the main performance issue.

@petemill petemill closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2023
@petemill petemill added closed/duplicate Issue has already been reported QA/No and removed QA/Yes labels Jun 13, 2023
@kjozwiak kjozwiak added release-notes/exclude and removed release-notes/include closed/duplicate Issue has already been reported labels Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/brave-news formerly brave-today OS/Android Fixes related to Android browser functionality OS/Desktop perf QA/No release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants