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

[Brave News]: RSS Feed Parsing Performance #29511

Closed
fallaciousreasoning opened this issue Apr 5, 2023 · 10 comments · Fixed by brave/brave-core#17923
Closed

[Brave News]: RSS Feed Parsing Performance #29511

fallaciousreasoning opened this issue Apr 5, 2023 · 10 comments · Fixed by brave/brave-core#17923

Comments

@fallaciousreasoning
Copy link

This is a followup to #29438 where we limited the number of feeds which could be parsed at the same time.

Looks like the root cause of the slowdown is that voca_rs is looking at graphemes when they shouldn't be (resulting in some O(N^2+) algorithms). As a temporary fix, I'm looking at manually stripping the HTML tags.

However, in future it would be good to do the following:

  1. Use the Browser HTML Parser (might be quite heavy duty).
  2. Don't parse the RSS feed in the browser process (we should probably do this in a sandboxed process).
@fallaciousreasoning
Copy link
Author

Instructions for testing are in @stephendonner's comment here: #29438 (comment)

@stephendonner
Copy link

stephendonner commented May 30, 2023

Verified PASSED using

Brave 1.53.74 Chromium: 114.0.5735.53 (Official Build) beta (64-bit)
Revision c499d7ea22c8b2dba278465a5df7b86a8efa4e64-refs/branch-heads/5735@{#970}
OS Windows 10 Version 22H2 (Build 19045.3031)

Steps:

  1. installed 1.53.74
  2. launched Brave
  3. skipped onboarding
  4. opened a new-tab page
  5. enabled Brave News
  6. loaded mate-desktop.org
  7. waited 30 seconds
  8. checked brave.exe CPU usage

Confirmed CPU usage was very low - reaching a high of 2-3%, idling slightly lower, on average

Task Manager process details
image image (1)

@stephendonner
Copy link

In addition, I also tested -- but didn't save/post results, here -- planet.mozilla.org and pravda.com, both of which have numerous feeds which also would've triggered this problem (and did), but now no longer do.

@hffvld
Copy link
Contributor

hffvld commented Jun 6, 2023

Verified on Pixel 7 using version(s):

Device/OS: Pixel 7 [panther_beta-user 14 UPB2.230407.019 release-keys]
Brave build: 1.53.81
Chromium: 114.0.5735.90 (Official Build) beta (64-bit)
Revision: 386bc09e8f4f2e025eddae123f36f6263096ae49-refs/branch-heads/5735@{#1052}

STEPS:

  1. Install Cpu Monitor app
  2. Launch Brave and Cpu Monitor in Split Screen Mode
  3. Open a new tab page > Enable Brave News
  4. Go to www.mate-desktop.org
  5. Wait ~30 seconds > Verify CPU usage
  6. Go to www.planet.mozilla.org
  7. Wait ~30 seconds > Verify CPU usage

ACTUAL RESULTS:

  • Verified that CPU usage is between 16% and 25% when the news feed is loading
1 2 3 4
1 2 3 4
timestamp_19-48-02_19-50-08.mp4

NOTE: When no apps opened, the lowest CPU usage was ~16%

@kjozwiak
Copy link
Member

Removing QA Pass-Win64 & QA Pass - Android ARM as the above will need to be rechecked via 1.52.x due to uplifting via brave/brave-core#18805. Used the verifications from #29511 (comment) & #29511 (comment) for the 1.52.x uplift rather than checking via Nightly as we usually do 👍

@kjozwiak
Copy link
Member

The above requires 1.52.125 or higher for 1.52.x verification 👍

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Jun 13, 2023

Verification PASSED on

Brave | 1.52.125 Chromium: 114.0.5735.110 (Official Build) (64-bit)
-- | --
Revision | 1c828682b85bbc70230a48f5e345489ec447373e-refs/branch-heads/5735_90@{#13}
OS | Windows 10 Version 22H2 (Build 19045.2965)

Steps:

  1. installed 1.52.125
  2. launched Brave
  3. skipped onboarding
  4. opened a new-tab page
  5. enabled Brave News
  6. loaded mate-desktop.org
  7. waited 30 seconds
  8. checked brave.exe CPU usage

Confirmed CPU usage was very low - reaching a high of 1%, idling slightly lower, on average

Task Manager process details
image image

@LaurenWags
Copy link
Member

LaurenWags commented Jun 13, 2023

Verified with

Brave | 1.52.125 Chromium: 114.0.5735.110 (Official Build) (x86_64)
-- | --
Revision | 1c828682b85bbc70230a48f5e345489ec447373e-refs/branch-heads/5735_90@{#13}
OS | macOS Version 13.4 (Build 22F66)

Steps:

  1. installed 1.52.125
  2. launched Brave, close and relaunch to pull griffin
  3. skipped onboarding
  4. opened a new-tab page
  5. enabled Brave News
  6. loaded mate-desktop.org
  7. waited 30 seconds
  8. checked Brave Browser CPU usage

Confirmed CPU usage was very low - reaching a high of 2-3%, idling slightly lower, on average

Browser Task Manager macOS Activity Monitor
1 2

For comparison, these are the results when running on 1.52.122, CPU is notably higher.

Browser Task Manager macOS Activity Monitor
1 2

@btlechowski
Copy link

btlechowski commented Jun 13, 2023

Verification passed on

Brave 1.52.125 Chromium: 114.0.5735.110 (Official Build) (64-bit)
Revision 1c828682b85bbc70230a48f5e345489ec447373e-refs/branch-heads/5735_90@{#13}
OS Ubuntu 18.04 LTS

Steps:

  1. installed 1.52.125
  2. launched Brave
  3. skipped onboarding
  4. opened a new-tab page
  5. enabled Brave News
  6. loaded mate-desktop.org
  7. waited 30 seconds
  8. checked brave.exe CPU usage

Confirmed CPU usage was very low - reaching a high of 2%, idling slightly lower, on average

image

@hffvld
Copy link
Contributor

hffvld commented Jun 13, 2023

Verified on Pixel 2 XL, Pixel 7 and Galaxy Tab S8 using version(s):

Device/OS: 
- Pixel 2 XL [taimen-user  8.1.0 OPM2.171026.006.H1 release-keys]
- Pixel 7 [panther_beta-user 14 UPB2.230407.019 release-keys]
- Galaxy Tab S8 [gts8wifixx-user 13 TP1A.220624.014 release-keys]
Brave build: 1.52.125 
Chromium: 114.0.5735.110 (Official Build) (64-bit)
Revision: 1c828682b85bbc70230a48f5e345489ec447373e-refs/branch-heads/5735_90@{#13}

STEPS:

  1. Install Cpu Monitor app
  2. Launch Brave and Cpu Monitor in Split Screen Mode
  3. Open a new tab page > Enable Brave News
  4. Go to www.mate-desktop.org
  5. Wait ~30 seconds > Verify CPU usage

ACTUAL RESULTS:

  • Verified that CPU usage is ~24% when the news feed is loading on Pixel 2
  • Verified that CPU usage is between ~29% when the news feed is loading on Galaxy Tab S8
  • Verified that CPU usage is ~21% when the news feed is loading on Pixel 7

Pixel 7 [Android 8]

timestamp_16-00-40_16-02-40.mp4

Galaxy Tab S8 [Android 13]

timestamp_15-53-21_15-55-12.mp4

Pixel 7 [Android 14]

timestamp_15-28-34_15-31-16.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment