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

Unrevert unified adblock catalog w/ perf fixes #33621

Closed
antonok-edm opened this issue Oct 13, 2023 · 3 comments · Fixed by brave/brave-core#21556
Closed

Unrevert unified adblock catalog w/ perf fixes #33621

antonok-edm opened this issue Oct 13, 2023 · 3 comments · Fixed by brave/brave-core#21556
Assignees
Labels
feature/shields/adblock Blocking ads & trackers with Shields OS/Android Fixes related to Android browser functionality OS/Desktop perf QA/No release-notes/exclude

Comments

@antonok-edm
Copy link
Collaborator

brave/brave-core#19946 was reverted by brave/brave-core#20442 due to a performance regression.

The root cause of the regression was that adblock components would generally update in succession, each one triggering a new callback to re-parse the adblock lists and load them into the engine. There is a lot of wasted work, exacerbated in the change to parse lists up-front rather than collecting them all first and parsing at the end.

The regression could be fixed by tracking pending component updates and waiting for all to be completed before reloading the engine. However, it is important to balance this with load at initial app startup, when initial versions of the catalog and lists are not necessarily ready yet.

@antonok-edm
Copy link
Collaborator Author

The performance here will also be generally improved as of brave/brave-core#20585

@LaurenWags
Copy link
Member

Adding QA/Blocked label per discussion with @kjozwiak - we have some questions about this one. @kjozwiak will start a thread in slack for discussion 👍🏻

@kjozwiak
Copy link
Member

Removing QA/Yes in favour of #35922 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/shields/adblock Blocking ads & trackers with Shields 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