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

fix(benchmark): convert cjs to nodenext #4465

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

seia-soto
Copy link
Member

This modernizes benchmarking source from commonjs to nodenext and fixes general type issues.

@seia-soto seia-soto added the PR: Internal 🏠 Changes only affect internals label Nov 24, 2024
@seia-soto seia-soto self-assigned this Nov 24, 2024
@seia-soto seia-soto requested a review from remusao as a code owner November 24, 2024 09:30
@seia-soto
Copy link
Member Author

I'm getting TS2742 from bench/micro.ts, maybe a fellow PR of exporting those components is unavoidable.

The inferred type of benchParsingImpl cannot be named without a reference to ../node_modules/@ghostery/adblocker/dist/esm/preprocessor.js . This is likely not portable. A type annotation is necessary.

@seia-soto seia-soto requested a review from chrmod November 24, 2024 09:34
@seia-soto seia-soto changed the title fix: convert from cjs benchmark to nodenext fix(benchmark): convert cjs to nodenext Nov 24, 2024
@seia-soto
Copy link
Member Author

Also, some fails (I need to investigate if this means the code execution is failed or not)

Micro Benchmark:
================
OK benchCosmeticsFiltersParsing 47 ~> 49 ops/sec (x1.04)
OK benchEngineCreation 12 ~> 13 ops/sec (x1.10)
OK benchEngineDeserialization 150 ~> 158 ops/sec (x1.05)
OK benchEngineSerialization 146 ~> 151 ops/sec (x1.03)
OK benchGetCosmeticTokens 146 ~> 148 ops/sec (x1.01)
FAIL benchGetCosmeticsFilters 865 ~> 845 ops/sec (x0.98)
FAIL benchGetNetworkTokens 124 ~> 120 ops/sec (x0.97)
FAIL benchNetworkFiltersParsing 69 ~> 65 ops/sec (x0.94)
FAIL benchRequestParsing 30317 ~> 29031 ops/sec (x0.96)
FAIL benchStringTokenize 90 ~> 86 ops/sec (x0.95)

seia-soto added a commit to seia-soto/adblocker that referenced this pull request Nov 26, 2024
seia-soto added a commit to seia-soto/adblocker that referenced this pull request Nov 26, 2024
@seia-soto seia-soto merged commit 9a35c59 into ghostery:master Nov 26, 2024
10 checks passed
@seia-soto seia-soto deleted the fix-benchmark branch November 26, 2024 10:20
chrmod pushed a commit that referenced this pull request Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Internal 🏠 Changes only affect internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants