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

Performance improvements #32

Open
wants to merge 1 commit into
base: prod
Choose a base branch
from
Open

Conversation

silto
Copy link

@silto silto commented Feb 26, 2024

Hello Defillama team!

I previously submitted a PR proposing the addition of a new tab page on this extension, with various other fixes/improvements. As discussed with @0xngmi , the fact that there is already a new tab page standalone extension and that adding this feature could be problematic for Chrome Web Store review, I decided to close the previous PR. But since I think my other performance improvements and code reorganization are still relevant, I submit to you this PR that contains only those. This introduces 0 changes to the end user, only changes that make the extension faster and the code (in my opinion) more maintainable.

Those are the changes :

IndexedDB/Dexie Databases

The phishing domains check uses IndexedDB managed by Dexie in the background script. The problem with this is that IndexedDB has a way of queueing requests that makes resolving results very slow during database updates (even when the db being updated is different from the one being read).
Some domain lists are pretty big (Metamask blocked lists is 97k entries), so the updates can last as long as 1min. You can notice the issue if you try to navigate when the extension has just been installed (and is going through its DB initialization). The logo will take a very long time to update when navigating to a new page.

New in this PR

The first approach that I implemented in my previous PR was to divide DB updates into chunks so the queries can come in between and be executed in reasonable time. The problem with this approach is that they can still take 2-10 seconds to resolve. So I added an additional optimisation, which consists in using the temporary arrays that are created by the update routine for searching the domain. Those arrays are in memory anyways while the script waits for the DBs to update, so using them doesn't have any cost. I also checked search time between those arrays and IndexedDB, and there is virtually no difference :

Capture d’écran 2024-02-26 à 01 02 09

Capture d’écran 2024-02-26 à 01 03 07

As you can see it's faster for small lists (allowed/fuzzy) and slower for the big blocked list, but we're talking 2ms vs 1ms, instead of 2-10 seconds.

Same as the Previous PR

I still kept update chunks (and increased their size to 1000) because it avoids doing massive DB updates with 100k+ lines, which is not recommended.

I also changed the way those updates are done, to avoid clearing the whole database every time. (this also prevents reading an empty database by accident).
Domains are now stored with an "updateId" key, that changes on each update. This way the update can bulkPut the domains that are fetched (overwriting existing ones and creating the new ones), and the old domains with an outdated updateId are removed after.

Background Script

I did various improvements to the Background script :

  • created a Background class to encapsulate all the logic
  • removed redundant startup script calls
  • proper alarm timing (refresh DBs every 4 hours instead of 1min, set in a config var)
  • removed useless phishing detector script calls (it was called every time a tab updated, even ones that weren't displayed, resulting in a lot of unnecessary checks)
  • moved DB interactions to handlers in libs/db.ts to avoid having db specific logic in the background script

Minor stuff

  • I moved some config to constant variables (DB update frequency, DB batch size, default configs)
  • updated package/manifest version
  • fix popup color mode not properly using the system config by default

…veness during db updates

reorganize background script into a class
move some values to constants
remove useless protocolsdb
fix light/dark mode in popup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant