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

Integrate @adguard/tsurlfilter #2109

Merged
merged 1 commit into from
Aug 4, 2021

Conversation

mjethani
Copy link
Contributor

@mjethani mjethani commented Aug 4, 2021

Let's integrate the @adguard/tsurlfilter library into the benchmarks.

@mjethani
Copy link
Contributor Author

mjethani commented Aug 4, 2021

@ameshkov

To try this out:

git clone -b integrate-tsurlfilter https://github.com/mjethani/adblocker
cd adblocker/packages/adblocker-benchmarks
npm install
make tsurlfilter

@ameshkov
Copy link
Contributor

ameshkov commented Aug 4, 2021

Submodule path 'blockers/ublock/submodules/uAssets': checked out '9df535fe7955996069377cbc63631a51aaeb72cd'
make -C ./blockers/ublock install-nodejs clean
tools/make-nodejs.sh
tools/make-nodejs.sh: line 61: eslint: command not found
make[1]: *** [dist/build/uBlock0.nodejs] Error 127
make: *** [node_modules/@gorhill/ubo-core] Error 2
error Command failed with exit code 2.

Requires globally installed eslint?

@mjethani
Copy link
Contributor Author

mjethani commented Aug 4, 2021

Requires globally installed eslint?

Sorry about this. The @gorhill/ubo-core integration is also a work in progress. Yes, it requires global ESLint.

@mjethani
Copy link
Contributor Author

mjethani commented Aug 4, 2021

@ameshkov use make tsurlfilter SHOW_MEMORY=1 to see information about memory usage (work in progress!). Use make brave, make ublock, make adblockplus, and make cliqz to run benchmarks for the respective content blocker.

@ameshkov
Copy link
Contributor

ameshkov commented Aug 4, 2021

@ameshkov use make tsurlfilter SHOW_MEMORY=1 to see information about memory usage (work in progress!). Use make brave, make ublock, make adblockplus, and make cliqz to run benchmarks for the respective content blocker.

Great, thank you, that's a great tool and we'll definitely be using it to measure the impact of the new changes.

On a side note, it probably makes sense to add info about requirements somewhere, I noticed it also requires rust.

@mjethani
Copy link
Contributor Author

mjethani commented Aug 4, 2021

… we'll definitely be using it to measure the impact of the new changes.

I can second this. I used it a lot for the optimization of Adblock Plus.

… probably makes sense to add info about requirements

Thanks! I'm trying to address the ESLint issue: gorhill/uBlock#3803 As for Rust, I guess we'll have to mention it in the requirements.

@mjethani mjethani marked this pull request as ready for review August 4, 2021 18:33
@mjethani
Copy link
Contributor Author

mjethani commented Aug 4, 2021

@remusao I guess it looks good to @ameshkov so I've marked it as ready for review. Let me know if you have any comments and then we can merge it.

@remusao remusao merged commit 7dbdbd4 into ghostery:master Aug 4, 2021
@mjethani mjethani deleted the integrate-tsurlfilter branch August 4, 2021 19:57
@mjethani
Copy link
Contributor Author

mjethani commented Aug 4, 2021

@ameshkov @remusao thanks for your time!

Note that we are also benchmarking serialization and deserialization and I have skipped this for @adguard/tsurlfilter to keep it simple for now.

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.

3 participants